Skip to content
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

CI: Publish workflow (PyPI) #83

Merged
merged 11 commits into from
Dec 3, 2024
Merged

CI: Publish workflow (PyPI) #83

merged 11 commits into from
Dec 3, 2024

Conversation

danielhollas
Copy link
Collaborator

@danielhollas danielhollas commented Nov 22, 2024

This adds a GHA workflow for publishing to PyPI.

This uses the relatively new Trusted Publisher authentication mechanism, which does not require storing a secret token in repository settings (we should consider migrating aiida-core to this as well imo).

The workflow itself is triggered when a tag is created (Typically by creating a release on GitHub UI). Additionally, one can test the release by publishing to TestPyPI by adding a test-release label to a PR.

I have used this workflow in another (non-AiiDA) repository and it's been working well.

Closes #72

@@ -1,3 +1 @@
"""A pytest plugin for testing AiiDA plugins."""

__version__ = '0.1.0.dev1'
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't think the __version__ attribute is very useful for this package and it complicates package management so I am removing it. Dissenters will be pointed to a lengthy discussion thread on CPython forum full of people arguing about this. :-D

Copy link
Member

Choose a reason for hiding this comment

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

Please point me to the lengthy discussion, would be a nice read material after dinner.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

pyproject.toml Outdated
@@ -7,8 +7,8 @@ name = "aiida_test_cache"

[project]
name = "aiida-test-cache"
version = "0.0.1a0"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I decided to first publish a version 0.0.1 which should be fully backwards compatible and can be used by existing users.

@danielhollas
Copy link
Collaborator Author

@unkcpz @agoscinski can you take a look? Once I have a green light on this workflow I'll add instructions for maintainers into docs.

@danielhollas
Copy link
Collaborator Author

@unkcpz @agoscinski Also can you please send me your user names on PyPI so that I can add you as maintainers of the aiida-test-cache package? Let me know if other people should be added. Thanks!

Copy link

@agoscinski agoscinski left a comment

Choose a reason for hiding this comment

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

Thanks for this PR @danielhollas . I myself was wondering recently how to do properly test releases in the CI. I think the PR is good as it is. My pypi account is alexgo (on test and regular). Just some questions:

Would be better to invoke the test release when pushing a tag to a branch that is not main? This way you also run check-release-tag for the test release.


This uses the relatively new Trusted Publisher authentication mechanism, which does not require storing a secret token in repository settings (we should consider migrating aiida-core to this as well imo).

Is it worth the effort? It seems just more convenient to set up but there seems to be no advantage when already having it set up.


@danielhollas
Copy link
Collaborator Author

Would be better to invoke the test release when pushing a tag to a branch that is not main? This way you also run check-release-tag for the test release.

I agree that this would be better, but I don't think it's possible, since in the Github actions there is no way to distinguish whether the tag is pushed to the main branch or different branch. Once the tag is pushed, the tag push event is fired and the workflow runs.

I've spent a lot of time trying to find a good solution for AiiDAlab repositories, but in the end did not end up with something fully satisfactory. For example, in aiidalab-widgets-base, we trigger the test release based on a branch name

https://github.com/aiidalab/aiidalab-widgets-base/blob/26773684a5b671bfcb9cee4ade248e7978bc26ff/.github/workflows/publish.yml#L11

a kind of like the label solution better, since it's kind of easy to forgot to name the branch appropriately.

Also in the above case, we trigger the actual release not a tag creation, but on publishing a release on GitHub.

https://github.com/aiidalab/aiidalab-widgets-base/blob/26773684a5b671bfcb9cee4ade248e7978bc26ff/.github/workflows/publish.yml#L13

The idea there was to avoid accidental release when someone would push a tag by accident.

In any case, for this small repo I wanted for the process to be as simple as possible so I think what we have here now is ok.

@danielhollas
Copy link
Collaborator Author

Is it worth the effort? It seems just more convenient to set up but there seems to be no advantage when already having it set up.

The main advantage is security, since long living secret tokens can be stolen. This is not a theoretical threat either, for example Codecov (which we use!) had a major breach couple years ago where attackers where targeting secrets on Github repos https://about.codecov.io/security-update/

I don't know how much effort it is to convert an existing project to this workflow. I can try to do it for one of the AiiDAlab repos and let you know.

@agoscinski
Copy link

Thanks for the detailed explanation. I am still not entirely sure if the trust relation between GitHub and the provider that needs to be established for the OIDC is not just moving the security problem to somewhere else but I guess people have thought this through.

@danielhollas danielhollas merged commit 4f67fae into main Dec 3, 2024
12 checks passed
@danielhollas danielhollas deleted the gha-publish branch December 3, 2024 18:38
@danielhollas danielhollas restored the gha-publish branch December 3, 2024 18:38
@danielhollas danielhollas deleted the gha-publish branch December 4, 2024 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Release automation
3 participants