Skip to content
This repository has been archived by the owner on Nov 3, 2023. It is now read-only.

Add github action to be able to release on published tag #582

Merged
merged 6 commits into from
Jan 2, 2023

Conversation

Pierre-Sassoulas
Copy link
Member

@Pierre-Sassoulas Pierre-Sassoulas commented Mar 10, 2022

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

@sigmavirus24
Copy link
Member

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

@Pierre-Sassoulas
Copy link
Member Author

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.

@sigmavirus24
Copy link
Member

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.

@Pierre-Sassoulas
Copy link
Member Author

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.

@sethmlarson
Copy link

sethmlarson commented Mar 11, 2022

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

@sethmlarson
Copy link

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.

@sigmavirus24
Copy link
Member

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

@Pierre-Sassoulas
Copy link
Member Author

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.

@sambhav
Copy link
Member

sambhav commented Apr 2, 2022

@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.

@sambhav
Copy link
Member

sambhav commented Apr 2, 2022

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! 🙌

@Pierre-Sassoulas
Copy link
Member Author

@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.

Copy link

@adamjstewart adamjstewart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

python -m pip install -U "setuptools>=56.0.0"
- name: Build distributions
run: |
python setup.py sdist bdist_wheel

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Direct invocation of setup.py is deprecated, current best practice is to use python -m build instead.

@Pierre-Sassoulas
Copy link
Member Author

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 :) )

@adamjstewart
Copy link

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.

@Pierre-Sassoulas
Copy link
Member Author

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

@sambhav
Copy link
Member

sambhav commented Jan 2, 2023

Thank you so much for this!

@sambhav sambhav merged commit f1dc7be into PyCQA:master Jan 2, 2023
@Pierre-Sassoulas Pierre-Sassoulas deleted the add-release-action branch January 2, 2023 20:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants