NLeSC / guide

Software Development Guide
https://guide.esciencecenter.nl
Creative Commons Attribution 4.0 International
47 stars 30 forks source link

Distributing python packages/releases through pypi and 2FA #290

Closed f-hafner closed 5 months ago

f-hafner commented 1 year ago

I'm not sure I'm right on this, but was wondering what others think. I recently tried to distribute a package through testpypi (will do pypi soonish), and followed the escience guide. I found it conflicts with the official documentation here: https://packaging.python.org/en/latest/guides/publishing-package-distribution-releases-using-github-actions-ci-cd-workflows/

there are two new things

With 2FA, it becomes difficult to publish manually with twine. There seems to be a way to do it, but it is not recommended:

STOP! You probably don't need this section; it exists only to provide some internal details about how GitHub Actions and PyPI coordinate using OIDC. If you're an ordinary user, it is strongly recommended that you use the PyPA's pypi-publish action instead.

In short, I suggest

Any thoughts? chapter owner @egpbos

bouweandela commented 1 year ago

I agree that we should recommend using CI to publish the packages because this 1) reduces the chances of including some rubbish that you happen to have in your local source code directory in the package and 2) is less work.

It would be nice to still mention that manually uploading is possible because sometimes you make a mistake in your CI configuration and that's difficult to test for a release workflow. It would be a bit sad if you need to make several releases just to fix your CI publishing workflow.

egpbos commented 1 year ago

I agree! Would you be willing to make a PR @f-hafner?

I'm a bit in doubt about mentioning manual uploading. For testing the CI publishing workflow you could also use testpypi (just quickly switch from pypi to testpypi in a PR, for instance). Then you immediately test your actual CI workflow, as opposed to again running the risk of reintroducing local rubbish.

f-hafner commented 1 year ago

yes, I'd be happy to make a PR!

As for the manual uploading: I agree to both arguments, but perhaps the manual way could be a "last resort" for debugging? There is also an option to specify verbose for debugging the upload to (test)pypi: https://github.com/marketplace/actions/pypi-publish#for-debugging

Here they publish to testpypi on each push, as it "is often used to indicate a healthy release publishing pipeline." -- but perhaps there are downsides to this that make it unattractive?

egpbos commented 1 year ago

Yes, that approach makes sense as well. I guess one could also pick manual for other reasons actually, like energy savings (avoiding spinning up a bunch of CI workers), so it's good to mention in any case.

Looking forward to the PR :)

egpbos commented 1 year ago

Note btw that I'm also updating slightly the releasing instructions in the Python template README.dev.md see https://github.com/NLeSC/python-template/pull/358 (commit coming in a minute).

bouweandela commented 1 year ago

For testing the CI publishing workflow you could also use testpypi (just quickly switch from pypi to testpypi in a PR, for instance).

I've been in the situation where the token for PyPI wasn't set up correctly, that's something you can't test.. maybe that won't happen anymore with the trusted publishing thing, but still, there is always the possibility to make a mistake somewhere when switching over.

f-hafner commented 5 months ago

I'm closing this as fixed in #323