Add Python 3 support #4
Conversation
|
This is a fix for #3 |
|
Tox output: |
|
@gobolt FYI |
| @@ -70,7 +72,10 @@ def compare(self, s1, s2): | |||
| def sign_string(self, key, text): | |||
There was a problem hiding this comment.
What type does sign_string return? Add to the comment and for corresponding sign_request method in request.py.
There was a problem hiding this comment.
Would you prefer if we returned either string or bytes depending on your python version, or just force the result to always be string?
Returning bytes in py3 would be the correct result imo, but it would complicate using the library.
| @@ -70,7 +72,10 @@ def compare(self, s1, s2): | |||
| def sign_string(self, key, text): | |||
| """ Return the signing method's digest """ | |||
| key, text = _clean(key), _clean(text) | |||
| return hmac.new(key, text, self.hash_fn).digest() | |||
| # Py3 hmac.new expects key as bytes. py2 expects str | |||
There was a problem hiding this comment.
_clean function suppose to convert Unicode to bytes. It is not correctly implemented. I think it is better we fix _clean instead of adding the second conversion level.
| return binascii.b2a_base64(binary).replace('\n', '') | ||
| # Py2 binascii.b2a_base64 gives us str, so str replacement | ||
| # Py3 gives us bytes, so we can only replace with bytes | ||
| newline = '\n'.encode() if PY3 else '\n' |
| # Py2 binascii.b2a_base64 gives us str, so str replacement | ||
| # Py3 gives us bytes, so we can only replace with bytes | ||
| newline = '\n'.encode() if PY3 else '\n' | ||
| empty = ''.encode() if PY3 else '' |
| @@ -14,7 +16,7 @@ def normalize(timestamp, nonce, method, path, host, port): # pylint: disable=R0 | |||
| """ Accepts args as strings and returns the normalized form for signing """ | |||
| normalized = '\n'.join([ | |||
There was a problem hiding this comment.
Here we make a string and decode nonce (which might be bytes, can it?). I think the only usage of normalized is in sign_string which requires bytes. If my understanding is correct, should not we just produce bytes here?
|
One of the recommendations on porting from Python 2 to 3 is to:
I found the advice very helpful in other projects I had to port. |
|
Closed in favor of #6 |
This adds python 3 support -- mostly around the string/bytes changes wrt digests.