Skip to content

Commit 3415669

Browse files
committed
Address review comments #2
Test for `basestring` instead of `str` and `unicode`. Clearer logging. Don't explicitly convert `Subscription` to a `str` for logging.
1 parent c7e1392 commit 3415669

File tree

1 file changed

+7
-6
lines changed

1 file changed

+7
-6
lines changed

code_comments/subscription.py

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ def select(cls, env, args={}, notify=None):
5353
criteria = []
5454
for key, value in args.iteritems():
5555
template = '{0}={1}'
56-
if isinstance(value, str) or isinstance(value, unicode):
56+
if isinstance(value, basestring):
5757
template = '{0}=\'{1}\''
5858
if (isinstance(value, tuple) or isinstance(value, list)):
5959
template = '{0} IN (\'{1}\')'
@@ -163,19 +163,20 @@ def _from_dict(cls, env, dict_):
163163
for _subscription in subscriptions:
164164
if subscription is None:
165165
subscription = _subscription
166-
env.log.debug(
167-
'Subscription already exists: %s', str(subscription))
166+
env.log.debug('Subscription already exists: [%d] %s',
167+
subscription.id, subscription)
168168
else:
169169
# The unique constraint on the table should prevent this ever
170170
# occurring
171-
env.log.debug('Multiple subscriptions found!')
171+
env.log.debug('Multiple subscriptions found: [%d] %s',
172+
subscription.id, subscription)
172173

173174
# Create a new subscription if we didn't find one
174175
if subscription is None:
175176
subscription = cls(env, dict_)
176177
subscription.insert()
177-
env.log.debug(
178-
'Subscription created: %s', str(subscription))
178+
env.log.debug('Subscription created: [%d] %s',
179+
subscription.id, subscription)
179180

180181
return subscription
181182

0 commit comments

Comments
 (0)