Skip to content

Add changes to build arm64 wheels #1108

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

Closed
wants to merge 1 commit into from

Conversation

tbbharaj
Copy link

@tbbharaj tbbharaj commented Jun 18, 2021

This PR addresses the issue to build arm64 wheels. To address this issue, I updated travis.yml to include arm64.

Also there were two additional errors (Master Build Error) I encountered when I triggered master build w/o any changes.

  1. On python: pypy2.7-6.0:
    Ref: https://travis-ci.com/github/tbbharaj/python-driver/jobs/515470491
Command "python setup.py egg_info" failed with error code 1 in /tmp/pip-install-9Ah7r6/lz4/
371The command "if [[ $TRAVIS_PYTHON_VERSION != pypy3.5 ]]; then pip install lz4; fi" failed and exited with 1 during .
  1. On Python: 3.7
    Ref: https://travis-ci.com/github/tbbharaj/python-driver/jobs/515470490
ERROR: InvocationError for command /home/travis/build/tbbharaj/python-driver/.tox/gevent_loop/bin/nosetests --verbosity=2 --no-path-adjustment /home/travis/build/tbbharaj/python-driver/tests/unit/io/test_geventreactor.py (exited with code -11 (SIGSEGV)) (exited with code -11)
939___________________________________ summary ____________________________________
940ERROR:   gevent_loop: commands failed
941The command "tox -e gevent_loop" exited with 1.

To fix above two errors, I made following changes:

  1. Update setup tools in travis.yml to fix build error in python setup.py egg_info,
  2. Update greenlet version in tox.ini to fix build error in tox -e gevent_loop

Successful builds Build

If there are any questions/comments - I'd be happy fix things as needed.
I opened the following JIRA issue as well Issue

Thank you
Tanveen

@tbbharaj tbbharaj force-pushed the tbbharaj/arm-build branch 2 times, most recently from 075b21e to 3eadf1a Compare June 30, 2021 21:26
Update setup tools to fix build error in `python setup.py egg_info`
Update greenlet version to fix build error in `tox -e gevent_loop`
@tbbharaj tbbharaj force-pushed the tbbharaj/arm-build branch from 3eadf1a to 18501e4 Compare July 1, 2021 01:37
@tbbharaj
Copy link
Author

Following up on this PR addressing the issue to build arm64 wheels.
Happy to address any feedbacks/comments.

Thank you!

@jdonenine
Copy link

Hi @tbbharaj thanks for the PR! We'll do our best to try to get someone to review the changes. Before we get going there ... Have you signed our CLA? https://cla.datastax.com/ -- we'll need you to do that before we can merge the PR.

Thanks!

@tbbharaj
Copy link
Author

Hi @tbbharaj thanks for the PR! We'll do our best to try to get someone to review the changes. Before we get going there ... Have you signed our CLA? https://cla.datastax.com/ -- we'll need you to do that before we can merge the PR.

Thanks!

Thank you Jeff, I haven't signed this CLA yet as I wasn't aware of it. I will work on getting approvals to get this CLA signed for merging this PR. I hope we can review the PR in the meantime.

@tbbharaj
Copy link
Author

@jdonenine Following up to check if we can have PR to get reviewed as we are working to get CLA approved. Thanks!

@absurdfarce
Copy link
Collaborator

@tbbharaj First off, thanks for the submission (and apologies for not updating this PR sooner)!

I am in the process of reviewing this PR now. We have a backend system which actually builds the wheels for packages that we push to pypi so there may be some changes we need to make here to account for that process. I'm having trouble convincing that backend process to correctly build ARM wheels, however, so until I get that sorted out I won't know for sure what changes we need to make on this PR. I'm working with the folks at Travis to try to resolve the issue and will try to do a better job of updating this PR as that process moves along.

@tbbharaj
Copy link
Author

@tbbharaj First off, thanks for the submission (and apologies for not updating this PR sooner)!

I am in the process of reviewing this PR now. We have a backend system which actually builds the wheels for packages that we push to pypi so there may be some changes we need to make here to account for that process. I'm having trouble convincing that backend process to correctly build ARM wheels, however, so until I get that sorted out I won't know for sure what changes we need to make on this PR. I'm working with the folks at Travis to try to resolve the issue and will try to do a better job of updating this PR as that process moves along.

No issues, Thank you for the update @absurdfarce. I will wait for next steps!

@absurdfarce
Copy link
Collaborator

Just updated https://datastax-oss.atlassian.net/browse/PYTHON-1278 with info about recent work on this effort. As mentioned above this ticket will be reviewed once we complete the work described there.

@absurdfarce
Copy link
Collaborator

@tbbharaj A quick update; I'm still waiting on a few internal infrastructure pieces to get the wheel building process back on track. In the meantime would you mind pulling your two build fixes (the upgrades to setuptools and greenlet) out of this PR and into their own PR? I'd like to get those changes into the Travis build as soon as possible so that every PR going forward will start from a baseline of a good, working build.

Thanks!

absurdfarce added a commit that referenced this pull request Sep 14, 2021
These fixes were originally implemented by user tbbharaj in #1108.
Extracting them into their own PR since 1108 is still being worked and I'd very much like to benefit from this
work across _all_ PRs against python-driver.

Major thanks to tbbharaj for the original work here.
absurdfarce added a commit that referenced this pull request Sep 15, 2021
These fixes were originally implemented by user tbbharaj in #1108.
Extracting them into their own PR since 1108 is still being worked and I'd very much like to benefit from this
work across _all_ PRs against python-driver.

Major thanks to tbbharaj for the original work here.
@absurdfarce
Copy link
Collaborator

@tbbharaj Just to make sure you were aware: I moved your TravisCI fixes into a new PR (#1111) and merged it into master. You'll probably want to merge master back into this branch at some point.

Thanks again for the fixes; it's great to see the TravisCI builds green again!

@absurdfarce
Copy link
Collaborator

@tbbharaj I just posted a comment to the JIRA ticket mentioning that the we now have a working public repository for wheel builds that includes the ARM platforms you wanted to add via this PR. As mentioned above I've also pulled out your fixes for the tests contained in the PR above and applied those separately. Thanks again for those; it's great to have a starting point of green test runs for future PRs.

Given all of that, I think I'm going to decline this PR itself. My rationale is that this PR moves wheel builds into the Travis runs for individual PRs which isn't really what we want to do... I'd rather keep those builds focused on running tests. More importantly this process has now been superseded by the public repository for Python wheels mentioned above.

Many, many thanks for pushing this process forward overall; I think we've landed in a better spot when it comes to our support for Python wheels (and the visibility and transparency of that support) and that's largely thanks to you kicking it off!

@absurdfarce absurdfarce self-requested a review September 17, 2021 17:27
@tbbharaj
Copy link
Author

@tbbharaj I just posted a comment to the JIRA ticket mentioning that the we now have a working public repository for wheel builds that includes the ARM platforms you wanted to add via this PR. As mentioned above I've also pulled out your fixes for the tests contained in the PR above and applied those separately. Thanks again for those; it's great to have a starting point of green test runs for future PRs.

Given all of that, I think I'm going to decline this PR itself. My rationale is that this PR moves wheel builds into the Travis runs for individual PRs which isn't really what we want to do... I'd rather keep those builds focused on running tests. More importantly this process has now been superseded by the public repository for Python wheels mentioned above.

Many, many thanks for pushing this process forward overall; I think we've landed in a better spot when it comes to our support for Python wheels (and the visibility and transparency of that support) and that's largely thanks to you kicking it off!

@absurdfarce Thank you so much... apologies for the delay on my end but it's great seeing this progress.
I see changes for aarch64 builds have been added here https://github.com/datastax/python-driver-wheels/blob/master/.travis.yml#L52. Is there a plan to make aarch64 wheels available on pypi.org soon?

@absurdfarce
Copy link
Collaborator

@tbbharaj Apologies, I've been working through a few other unrelated issues so I'm running a bit behind here. It certainly is my intent to deploy the ARM wheels to pypi.org in the near future. I'll be sure to post an update here once that's been done.

Thanks!

@tbbharaj
Copy link
Author

@tbbharaj Apologies, I've been working through a few other unrelated issues so I'm running a bit behind here. It certainly is my intent to deploy the ARM wheels to pypi.org in the near future. I'll be sure to post an update here once that's been done.

Thanks!

Thanks for the update @absurdfarce

@absurdfarce
Copy link
Collaborator

@tbbharaj Apologies for the delay, but I come bringing good news. I just deployed the ARM wheels for cassandra-driver to Pypi! I pushed wheels for Python 3.7 and 3.8; they should be visible at https://pypi.org/project/cassandra-driver/#files.

My guess is that you have a testbed for ARM installs that's far more accessible than what I have access to at the moment so are you able to confirm that (a) Python is correctly downloading these wheels when using cassandra-driver on an ARM platform and (b) the package seems to function normally when you do so?

Thanks again for your patience while we work through these changes!

@tbbharaj
Copy link
Author

@tbbharaj Apologies for the delay, but I come bringing good news. I just deployed the ARM wheels for cassandra-driver to Pypi! I pushed wheels for Python 3.7 and 3.8; they should be visible at https://pypi.org/project/cassandra-driver/#files.

My guess is that you have a testbed for ARM installs that's far more accessible than what I have access to at the moment so are you able to confirm that (a) Python is correctly downloading these wheels when using cassandra-driver on an ARM platform and (b) the package seems to function normally when you do so?

Thanks again for your patience while we work through these changes!

@absurdfarce Thats great news! Thank you so much...I am closing this PR for now then :)

@tbbharaj tbbharaj closed this Oct 12, 2021
@absurdfarce absurdfarce mentioned this pull request Oct 19, 2021
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