-
Notifications
You must be signed in to change notification settings - Fork 314
Publish wheels and sdist in PyPI #1099
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
Publish wheels and sdist in PyPI #1099
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1099 +/- ##
=======================================
Coverage 82.78% 82.78%
=======================================
Files 75 75
Lines 3306 3306
=======================================
Hits 2737 2737
Misses 569 569
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
| matrix: | ||
| os: [ubuntu-latest, windows-latest, macos-latest] | ||
| python-version: [2.7, 3.7, 3.8, 3.9] | ||
| python-version: ['2.7', '3.7', '3.8', '3.9'] |
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.
I took the opportunity to use strings here because without quotes these were floats and a future 3.10 would have been interpreted as 3.1.
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.
nice catch!
|
This is amazing. Thanks so much for setting this up @JeanChristopheMorinPerso ! I don't fully understand the risks of the less safe option. Is the risk that the released package could be broken due to drift outside the OTIO source code between the two builds? The time difference is likely to be very small, since we often land a few changes right before a release milestone. Is there any security risk in there too? When we have done pypi package releases in the past, we did them to the test pypi index first, verified them, then pushed to the official one. Is there a similar step here where we can vet the final package before releasing it to the world? Perhaps that would come included in the safer approach you mentioned. |
|
This is awesome! @jminor I think the 'verification' is less important here because in a way, the GitHub action itself is doing the verification -- the tests are running and passing and so on, using the wheels that will be uploaded to pypi, right? |
|
It looks like it builds an sdist separately - that is for platforms that don't have compatible wheels, correct? |
Exactly. It's also to be a good citizen for systems consuming sdists (for example Linux distros tend (at least some of the are) to repackage popular PyPI packages and they normally use the sdists). |
|
Ok, I really like this. Thanks so much for submitting it! If you have a little time, do you think you could make one (temporary) change, so that we can test this out? In this doc: https://github.com/marketplace/actions/pypi-publish#advanced-release-management They note a way of having the action upload to the test server on every commit rather than just to the main one on publish. Could we turn that on for this PR so that we could see the result? Then turn it back off before landing it with the on-release-tag-only switch for actual usage? If that seems feasible we can coordinate about setting up the tokens and so on. What do you think? |
|
Alternatively, it might just make sense to publish packages from commits on master to the test pypi. Is that commonly done? |
Exactly this yes. On Windows and macOS the compiler could be updated, etc.
I don't believe so.
Not yet unfortunately. We don't run the tests against the produced wheels. To run the tests, we run
The "safer" approach is similar to what you described indeed. |
|
@ssteinbach I can certainly set the URL to the test PyPI. Just let me know once the credentials are setup in the repo and I'll do the change.
As far as I know it's uncommon. Though what we could have is a manual action where you specify a build ID (https://github.blog/changelog/2020-07-06-github-actions-manual-triggers-with-workflow_dispatch/) and it would take care of publishing in test.pipy.org or something similar. |
|
I will close this and re-open a new pull request later with new changes. |
Publish wheels and sdist (source code only) in PyPI.
Fixes #988 .
The method I used is simple. I added a job in the
python-packageworkflow that will only run when the GitHub Action is triggered by tag pushed in the repo.That's the simplest option. But it has one major downside. The artifacts will be regenerated for the release. This is a problem because there could be some amount of time between the last PR is merged and the tag creation and during this time some things might have changed (transitive dependencies, GitHub Actions environment, etc). On Linux it isn't much a problem as wheels are generated in docker images, but on macOS and Windows it's another story.
A safer approach would be to have the publish job be in its own workflow. This workflow would fetch the wheels from the last successful build (or a specific build if we make the workflow that can be manually triggered with custom inputs). This is the approach the the Java bindings is using, with the difference that the publish workflow is triggered by a release creation, not a tag pushed.
I will be happy to go with later (safer) approach if TSC prefers that.
Notes
PYPI_API_TOKENto the repository. The value needs to be a PyPI token created from the user interface (see https://pypi.org/help/#apitoken for instructions). You will need to specify a name when creating the token. I suggest choosing a name that clearly states that it's going to be used by GitHub actions .