pydocstyle icon indicating copy to clipboard operation
pydocstyle copied to clipboard

Add github action to be able to release on published tag

Open Pierre-Sassoulas opened this issue 2 years ago • 11 comments

This permit to release on pypi if a value for PYPI_API_TOKEN is set in github settings. The release is done when a release is created/published in github interface.

Relates to #575

Pierre-Sassoulas avatar Mar 10 '22 19:03 Pierre-Sassoulas

This should be a protected workflow so only trusted folks can do this, no?

sigmavirus24 avatar Mar 10 '22 22:03 sigmavirus24

This should be a protected workflow so only trusted folks can do this, no?

The workflow is triggered by creating a release (in github interface from this url https://github.com/PyCQA/pydocstyle/releases) so only members that can release a package can trigger this workflow.

Pierre-Sassoulas avatar Mar 10 '22 22:03 Pierre-Sassoulas

I wasn't sufficiently clear. There are often cases where you don't want everyone with permission to create a release to be able to publish to PyPI. No one can pip install from GitHub and get malicious code (although some people apparently do install artifacts from there which boggles my mind). Publishing to PyPI should be more severely restricted. I thought @sethmlarson had written a blog post about this but I'm not seeing it.

sigmavirus24 avatar Mar 11 '22 12:03 sigmavirus24

I don't see the purpose of a release without a pypi publication alongside it, so for me those permissions were equivalent. Let me know if I need to modify something :)

some people apparently do install artifacts from there which boggles my mind

I would have made the mistake 15 years ago. It's probably autodidacts with no formal training who were never introduced to package management. When you come from a Windows background you're used to searching for an executable to install after searching for the name of the software online, and github releases are what's closest to this.

Pierre-Sassoulas avatar Mar 11 '22 12:03 Pierre-Sassoulas

@sigmavirus24 I think this is the article you're referencing? https://sethmlarson.dev/blog/security-for-package-maintainers

sethmlarson avatar Mar 11 '22 13:03 sethmlarson

Regarding this PR, creating a Github release only requires write permissions, instead use a GitHub Environment to create a "reviewable" execution of a Github Action. The article doesnt cover this as it's a whole other topic on how to configure deployment pipelines and project hosts.

sethmlarson avatar Mar 11 '22 13:03 sethmlarson

for me those permissions were equivalent

They shouldn't always be equivalent. That's my point. Also, GitHub releases are trash for so many reasons and create extra burden on maintainers. They shouldn't be the vehicle to publishing to PyPI

sigmavirus24 avatar Mar 12 '22 12:03 sigmavirus24

It's also possible to launch a github actions on tag creation directly but there would still be the problem of who has right to create tag. If you don't want to mix pypi / github rights then the solution is probably a local script launched by someone with pypi's release right.

Pierre-Sassoulas avatar Mar 12 '22 12:03 Pierre-Sassoulas

@Pierre-Sassoulas - I have created two environments pypi-dev (which has tokens for test.pypi) and pypi-prod (which has tokens for pypi) both of which are gated and require approvers from the current admins (me and @Nurdok) who have release permissions on PyPi. Could you follow something similar to https://timheuer.com/blog/add-approval-workflow-to-github-actions/ to add these environments to the workflow?

I have created project specific tokens and added them as env vars to these environments.

samj1912 avatar Apr 02 '22 17:04 samj1912

Separately - it would have been great if we could have versions also driven by the Github tags instead having to manually bump them similar to https://github.com/cruft/cruft/blob/0c6de85d974197969c0e65913857eaa36b788e5e/.github/workflows/pypi_publish.yml#L22

but any improvement to the release process is great. Thanks for this PR! 🙌

samj1912 avatar Apr 02 '22 17:04 samj1912

@samj1912 I don't know if I understood your first link correctly, Would 342cc17 limit the person that can launch the job ? In order to test we need to create a tag and publish a release which is... something that I'd try on another smaller repo, personally 😄

Second link look nice too but I have very limited available time and I'm not knowledgeable enough about poetry right now.

Pierre-Sassoulas avatar Sep 15 '22 18:09 Pierre-Sassoulas

Most of the projects I maintain use pypa/cibuildwheel and actions/upload-artifact instead of manual build and twine. May be more reliable since they are maintained by PyPA or GitHub themselves.

This looks pretty cool, are wheel properly created for al operating systems with python -m build ? Because otherwise we should definitely use what you suggested. (I'm using this release workflow in a number of places so I appreciate the review, thank you @adamjstewart :) )

Pierre-Sassoulas avatar Jan 02 '23 19:01 Pierre-Sassoulas

are wheel properly created for al operating systems with python -m build?

In the last couple of years, PyPA completely overhauled their support for non-default build backends. Build "backends" include things like distutils, setuptools, flit, hatchling, poetry, etc. Build "frontends" include things like build and pip. One of the advantages of build and pip is that they support all of the above build backends, whereas setup.py only supports distutils/setuptools. Also setup.py is deprecated as I mentioned.

python -m build basically does the same thing that python setup.py build used to do. For pure-Python projects like pydocstyle, it creates a single universal wheel for Python 3. For Python libraries that depend on external libraries or that require compilation, it builds a Python-version and platform-specific wheel, and you need to run this command on all versions/platforms (e.g., via matrix in GitHub Actions) and upload each one separately.

cibuildwheel is probably overkill for this project since we don't need a separate wheel for each Python version/platform. Let's stick with build/twine instead.

adamjstewart avatar Jan 02 '23 19:01 adamjstewart

Thanks a lot for the explanation, I only have pure python project right now but I'll keep that in mind 👍

Pierre-Sassoulas avatar Jan 02 '23 19:01 Pierre-Sassoulas

Thank you so much for this!

samj1912 avatar Jan 02 '23 20:01 samj1912