-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support Python 3 #2833
Support Python 3 #2833
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2833 +/- ##
==========================================
+ Coverage 84.69% 88.54% +3.85%
==========================================
Files 662 7 -655
Lines 37611 262 -37349
Branches 4507 40 -4467
==========================================
- Hits 31853 232 -31621
+ Misses 4436 19 -4417
+ Partials 1322 11 -1311 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Left two small comments.
import requests | ||
|
||
# project | ||
from six.moves.urllib.parse import urlparse | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small nit, but looks like an extra newline here.
@@ -72,14 +73,15 @@ def _get_data(self, url, instance): | |||
'ssl_certfile': instance.get('ssl_certfile'), | |||
'ssl_verify': instance.get('ssl_verify'), | |||
} | |||
for key, param in ssl_params.items(): | |||
ssl_params_copy = copy.copy(ssl_params) | |||
for key, param in iteritems(ssl_params_copy): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for key, param in iteritems(ssl_params_copy): | |
for key, param in list(iteritems(ssl_params)): |
We could remove the copy
line then. Using list()
is supposed to be slightly faster as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! 🐍3️⃣
What does this PR do?
Moves GoExpvar to Python3! This one was a straightforward one. I was worried about that, but it seems like the string manipulation issues are mostly handled in this one.
Motivation
🐍 3️⃣ 🚂
Review checklist
no-changelog
label attachedAdditional Notes
Anything else we should know when reviewing?