Skip to content
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

Chore/python3 #39

Merged
merged 14 commits into from
Jul 2, 2019
Merged

Chore/python3 #39

merged 14 commits into from
Jul 2, 2019

Conversation

paulineribeyre
Copy link
Contributor

@paulineribeyre paulineribeyre commented Jun 26, 2019

  • ran 2to3 for python 2 to 3 syntax updates
  • ran black on everything and added wool
  • encoding fixes

Breaking Changes

  • Python 3 instead of python 2

@PlanXCyborg
Copy link

The style in this PR agrees with black. ✔️

This formatting comment was generated automatically by a script in uc-cdis/wool.

Copy link
Contributor

@fantix fantix left a comment

Choose a reason for hiding this comment

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

Sorry about all the trivial comments

@@ -7,8 +7,9 @@
try:
from urllib.parse import urlparse, parse_qs, quote, unquote, quote_plus
except ImportError:
from urlparse import urlparse, parse_qs
from urllib import quote, unquote, quote_plus
Copy link
Contributor

Choose a reason for hiding this comment

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

This was Python 3 compatible, 2to3 is not smart enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh, right, i will just remove the try except since we don't need to support python 2 anymore

else:
# Grab the key from the first in the list of keys.
return flask.current_app.jwt_public_keys.items()[0][1]
return list(flask.current_app.jwt_public_keys.items())[0][1]
Copy link
Contributor

Choose a reason for hiding this comment

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

How about:

next(iter(flask.current_app.jwt_public_keys.values()))

@@ -224,7 +224,7 @@ def get_canonical_headers(req, include=None):
# Requests, since it uses a case-insensitive dict to hold headers, this
# is here just in case you duck type with a regular dict
cano_headers_dict = {}
for hdr, val in headers.items():
for hdr, val in list(headers.items()):
Copy link
Contributor

Choose a reason for hiding this comment

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

useless list()

@@ -280,7 +280,7 @@ def format_cano_path(path):
qm = b'?' if PY2 else '?'
full_path = qm.join((full_path, qs))
if PY2:
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we still need these if PY2?

@@ -167,7 +161,8 @@ def auth_header(encoded_jwt):
Return:
List[Tuple[str, str]]: the authorization header
"""
return [('Authorization', 'Bearer %s' % encoded_jwt)]
encoded_jwt = encoded_jwt.decode("utf-8")
Copy link
Contributor

Choose a reason for hiding this comment

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

inline variable?

return auth_str


def generate_signature(secret_key, sig_string):
if isinstance(secret_key, str):
secret_key = bytes(secret_key, "latin-1")
Copy link
Contributor

Choose a reason for hiding this comment

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

secret_key.encode("latin-1")? Identical but consistent 🙊

.travis.yml Outdated
@@ -25,7 +29,7 @@ deploy:
env:
global:
- PIPENV_IGNORE_VIRTUALENVS=1
- secure: ov66mTdCG+Gl+wLAjYdqR0HrKbgpQFSxSgCi0ihTmErzQl/vudZyPXdoSEQfFsWpKGI+UmhYklsvGySnYzyhTuUTrmovQ5C9tMMASWtJrwzAgun6PIli7ETjw5j273WTwIPy/HfrysqW6ab563VYbbuxn7ZANV86wN17TKp0qyaJPD4Kc6iFp8yKei7msXPy6Pc36AYz4vVjq1AZasFLIrRy5va4VNxtNUvK4peOdKcxDTmj4GMS8eF/Y55HuOzuH/B3hw0XqCgwk+u+4DCImpvQIhsfChgI71m70/scWwhiSaVY3WEmZBbJPgFLsoBm+div2+kG8zcNvOK2OEhQgvocxzgpTik5rvZX2SRCpuBxgbZEzUEbvNPArJVLwOBlbadIpuM8+sHTj/2A/4+AboVat10jjHrevTKlCo6CXe2QPXO474eZP0f5lOsGRQ9iSW52MRRCt8yOIALWwmn5DDrwt41ezn5+JDglxPxebqFLuvutG/7XMhhCuU91MYLxI/xhliRcYL9SiZXjnEz0p91YUNQQ2seJNvw/d65ypYdJDqS8kM8II7+lL/9l41hZ2En3LDSQ5diXPzEeNVp5cz/mn2rwlehTLmGbdb0Qy3WbD9eKoY1ADR+yDTE9l3t9UtRqkJ6Jgdig1YNkmdaFmrMfMHzTntY5He78b95zyk8=
- secure: qoMlJG2/e/ExVimSYtWbFxaePA7wSUgjsENaJPmAZKvisaSmbh3+BjlGc0128NViYGXMg2ljk2ofzST7/ktlJhnfewFrVHMctJsDppC0cmYXYQsXdGlFC59UDZXzp+jj4dRVSBiFUbIcznQDuGQ9z9rZs/dAHxPyVfgZoKbvuAiW+vNh+9veVvMfTFdSl6xNC4VBAk1BSWwRgYzYRyUFCXTmX+LbKN6SbyXpZXEwHoLCzKGePDK4SOHeFMSVhPryWtID1aCZYHHRm1ctFTjgvuAJ6s58Tlui5K9w4LFzlyhbWPE/T42cO1Azuqm+T/UZkD9k//rIj6yZb+JuxTNaIhosmB83MYzD/XhOEqCKoUjvYAHT1FwVpuml8/lm16EEloerag6yudAuVwerOQtPp3bcYANtKy78dLtVbu6AUDlpgjy3TELR60n02JDP2C5eu/3Bf2yLY3zOgshWluq6pTgX1PDaBgVL6PwDjnGtWI4Xpblt56rCYKvqHVqkQJzmTGP6epooiNEKaWy9tSo4aHRw82uzqOJY7Y3vr2eJ1vUtV5MBTSDnZyclRvEJA+qB6vHHSO7FRzTnzdvRmCO2VFOF+VfRGk8p331bBhotHIKxu5SwYxG97L8QuS4H9yni9zw039tCqluj2sEZ2xER/2AWCFtayty5ldz1SXIyY6U=
Copy link
Contributor

@fantix fantix Jul 1, 2019

Choose a reason for hiding this comment

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

I cannot decrypt, but I think it is a change like this:

-GH_TOKEN=xxxx
+GITHUB_TOKEN=xxx

We actually need both GH_TOKEN - gen3git needs the first one (perhaps it is even better to make gen3git support both)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right 🤦‍♀

@paulineribeyre paulineribeyre merged commit 7ce54fd into master Jul 2, 2019
@paulineribeyre paulineribeyre deleted the chore/python3 branch July 2, 2019 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants