-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
[BEAM-1251] Upgrade from buffer to memoryview (again) #5887
[BEAM-1251] Upgrade from buffer to memoryview (again) #5887
Conversation
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.
LGTM
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.
Thank you, this is a great change! This LGTM. I resolved the merge conflict and will merge after tests pass.
Heads up that this change caused issues in postcommits. Will investigate more in the morning.
https://builds.apache.org/job/beam_PostCommit_Python_Verify/5529/consoleFull |
Root cause is the following. Looks like we need to add "python-snappy == 0.5.3" into the setup.py file.
|
The build seems to be broken after this. Can you please take a look. Thannks |
We should roll back until we figure out a proper fix. Unfortunately, we can't just add a python-snappy pin to the setup file, since it doesn't compile on all platforms. |
Is this the only commit that needs to be reverted? |
@angoenka Yes |
This PR is a second shot at #4820 based on the work that @pitrou has done to allow python-snappy to support Python memoryviews. This has in turn has enabled @martindurant to release python-snappy 0.5.3 to PyPI.
This PR recommends the use of that release to upgrade from buffer to memoryview in avroio.py because buffer was removed in Python 3 in favor of memoryview which is supported in all Python versions that Beam supports.
Reviews please from @aaltay @holdenk @superbobry
flake8 testing of https://github.com/apache/beam on Python 3.6.3
$ flake8 . --count --select=E901,E999,F821,F822,F823 --show-source --statistics
Follow this checklist to help us incorporate your contribution quickly and easily:
[BEAM-XXX] Fixes bug in ApproximateQuantiles
, where you replaceBEAM-XXX
with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.It will help us expedite review of your Pull Request if you tag someone (e.g.
@username
) to look at it.Post-Commit Tests Status (on master branch)