-
Notifications
You must be signed in to change notification settings - Fork 34
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
Add support for python3 #69
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…nges even on failure Signed-off-by: Justin Israel <[email protected]>
Signed-off-by: Justin Israel <[email protected]>
Signed-off-by: Justin Israel <[email protected]>
Signed-off-by: Justin Israel <[email protected]>
Signed-off-by: Justin Israel <[email protected]>
Signed-off-by: Justin Israel <[email protected]>
Signed-off-by: Justin Israel <[email protected]>
…old unittest backport Signed-off-by: Justin Israel <[email protected]>
Signed-off-by: Justin Israel <[email protected]>
Signed-off-by: Justin Israel <[email protected]>
Signed-off-by: Justin Israel <[email protected]>
Signed-off-by: Justin Israel <[email protected]>
Signed-off-by: Justin Israel <[email protected]>
Signed-off-by: Justin Israel <[email protected]>
Signed-off-by: Justin Israel <[email protected]>
Signed-off-by: Justin Israel <[email protected]>
Signed-off-by: Justin Israel <[email protected]>
Thanks for merging. It this going to get a new minor tag? |
v0.0.8 is yours. Thanks for the hard work. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a large merge which adds support for python 3.6+
It was a bit time consuming and took 3 full days. Some of the time was spent improving the tests and a good majority was spent dealing with the
bytes
/str
handling between the api and the underlying sockets, buffers, and encoder/decoder.Summary of changes:
bytes
in py3I had to make a decision on the option for handling
bytes
vsstr
between python 2 and 3, and opted to keep a compatible experience for existing py2 users. And in py3 I went with encoding to bytes at the buffer/socket layer to allow forstr
type at the api layer. I had also encountered a new situation in my porting experience, where dictionaries unmarshaled directly from bytes produce bytes format keys. This meansb'key' != 'key'
. So I went with updating the codegen so the spec will decode the keys to a more naturalstr
type.Caveats:
There are 4 tests that originally failed for me in python2 using the master branch or 0.0.7 release tag (tested against RabbitMQ 3.6.6)
For the qos tests, I was finding that the failure happened after the setting of the
basic_qos
, when the expected remaining messages were not read.For the delete tests, I was finding that the expected
NotFound
exception no longer raises when deleting a queue or exchange that does not exist.I have not spent time investigating these tests since they have not impacted our own higher level library consumption of puka.
Fixes #7
Refs #68