Skip to content
This repository was archived by the owner on Jul 11, 2022. It is now read-only.

python3 compat, hasattr iteritems->items #113

Merged
merged 1 commit into from
Jan 14, 2018

Conversation

kbroughton
Copy link
Contributor

I'm not sure what data structures this test was for, but for python3 it fails because dict has no iteritems. Changes were made around it for six.iteritems, but this change was missed.

Once i made this change (and another PR to django_opentracing), i was able to run django_opentracing on django python3

@kbroughton kbroughton changed the title python3 compat, hasattr iteritems->itemx python3 compat, hasattr iteritems->items Jan 13, 2018
@coveralls
Copy link

Coverage Status

Coverage remained the same at 93.904% when pulling a65020d on kbroughton:master into fdf2778 on jaegertracing:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 93.904% when pulling d05b1c6 on kbroughton:master into fdf2778 on jaegertracing:master.

@@ -82,7 +82,7 @@ def inject(self, span_context, carrier):
carrier[header_key] = encoded_value

def extract(self, carrier):
if not hasattr(carrier, 'iteritems'):
if not hasattr(carrier, 'items'):
Copy link
Member

@yurishkuro yurishkuro Jan 14, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not able to find the reason why this code was checking for iteritems instead of isinstance(carrier, dict) as done in the inject method, but I seem to recall there was a reason for it, i.e. a type was passed in some cases that wasn't a true dict but allowed iteritems (with there was a unit test).

@yurishkuro yurishkuro merged commit 05cfefa into jaegertracing:master Jan 14, 2018
@yurishkuro yurishkuro mentioned this pull request Jan 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants