Devops: Add trigger for release workflow to TestPyPI#6891
Devops: Add trigger for release workflow to TestPyPI#6891agoscinski merged 2 commits intoaiidateam:mainfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6891 +/- ##
==========================================
+ Coverage 79.13% 79.14% +0.02%
==========================================
Files 565 565
Lines 43391 43391
==========================================
+ Hits 34331 34339 +8
+ Misses 9060 9052 -8 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
danielhollas
left a comment
There was a problem hiding this comment.
Thanks! I also felt this need in other repositories, but I don't think using git tags is the best we can do here and is quite clunky. Supposedly you have to manually create (and then delete?) the tag?
In other projects like cpython or ruff I saw they are using PR labels for this, and I think that's quite elegant. I implemented that a while ago for the aiida-test-cache and was happy with how that worked. Please see
https://github.com/aiidateam/aiida-test-cache/blob/main/.github%2Fworkflows%2Fpublish.yml
I don't fully see yet why adding and removing a PR label is more elegant than adding and removing a tag (to retrigger the release CI). It seems to me like a preference if one does like more 2-4 button clicks in the browser or 3 commands in the terminal (tag delete, new tag, tag force push). I see an argument for reusing the existing logic for tags since reusing logic reduces maintenance. |
The main reason is that the label is localized to the PR itself, while the git tag is attached to the git repo itself (and is visible in various places in github UI). For for example if somebody happened to be cloning the repo in the middle of this they could pull the extra test tag as well.
That to me is a strong argument against -- I'd rather have a bit of duplication and have those paths be completely separate to minimize the chance that we'd publish a release by accident. |
Which represents what it should be, one commit should only run the test release CI. If one forgets to delete then its really not horrible.
That can also happen when duplicating the workflow. The difference can be one environment variables in the test workflow that publishes to pypi instead test.pypi. I think it depends more on how you structure the workflow file rather than the type of mechanics. My main motivation is to make the CI pipeline not more complicated by adding another github action plus additional github events that are only required for this one workflow so one needs to read how they work and needs to maintain them if they fail. It is also another mechanic one has to learn for doing releases. The CI is already complicated enough that not many people can maintain it well. I had a closer look at the PR. If we do not use the remove label action, I think it is okay in complexity. I would nevertheless still reuse the workflow steps that test the package before release for both test.pypi and pypi. |
|
Note for me, we can keep the workflow be retriggered by commit as long as the label is on it by using on:
pull_request:
branches-ignore: [gh-pages]
paths-ignore: [docs/**]
env:
FORCE_COLOR: 1
jobs:
check-release-tag:
# Only run this job on the main repository and not on forks
if: >-
github.repository == 'aiidateam/aiida-core' &&
github.event_name == 'pull_request' &&
contains(fromJson(toJson(github.event.pull_request.labels)).*.name, 'test-release')
TODO get test pypi access and figure out how to reuse workflow without doing |
24d3deb to
6682e50
Compare
|
So I use now the label method. You can see a CI run of PR #6854 here https://github.com/aiidateam/aiida-core/actions/runs/15727580379 Note that I intentionally want this to be run on each push event to a PR as typically one wants to debug the release workflow requiring multiple commits, reapplying the label is very annoying and spams the PR history. Since one can delete releases on test pypi without consequences, and one just remove the label to not trigger it anymore I think this is the way with the least friction (at least for me). |
There was a problem hiding this comment.
I feel like it would be much simpler to create a new yaml file with just for the test publish workflow. It would simplify the if statements, and it would be less error-prone, and there wouldn't even be much duplication, since you just need the new publish-testpypi step, which doesn't really need pre-commit and test jobs.
|
|
||
| name: Publish to test PyPI | ||
|
|
||
| needs: [pre-commit, tests] |
There was a problem hiding this comment.
If you're publishing on TestPyPI, you don't really care about the tests right?
| needs: [pre-commit, tests] | |
| needs: [pre-commit] |
There was a problem hiding this comment.
I want to have the same workflow run for testpypi release as for pypi release (as much as possible since checking tag does not work), so the official release pipeline does not fail because something was not considered.
.github/workflows/release.yml
Outdated
| # Only run this job on the main repository and not on forks. | ||
| # and only run it if a tag is pushed or if the test-release label is active. | ||
| # When the tag is released we will publish to pypi. | ||
| # When the a label is active we will publish to testpypi |
There was a problem hiding this comment.
This comment does not really correspond to the if statement below it.
There was a problem hiding this comment.
Yes the logic is distributed because of the github workflow syntax. I put the comment on top.
There was a problem hiding this comment.
| # Only run this job on the main repository and not on forks. | |
| # and only run it if a tag is pushed or if the test-release label is active. | |
| # When the tag is released we will publish to pypi. | |
| # When the a label is active we will publish to testpypi |
There was a problem hiding this comment.
I refactored the doc at the top
.github/workflows/release.yml
Outdated
| # Only run this job on the main repository and not on forks. | ||
| # and only run it if a tag is pushed or if the test-release label is active. | ||
| # When the tag is released we will publish to pypi. | ||
| # When the a label is active we will publish to testpypi |
There was a problem hiding this comment.
| # Only run this job on the main repository and not on forks. | |
| # and only run it if a tag is pushed or if the test-release label is active. | |
| # When the tag is released we will publish to pypi. | |
| # When the a label is active we will publish to testpypi |
fddb52d to
b631811
Compare
b631811 to
e615cf6
Compare
|
@danielhollas only change is doc e615cf6 |
By using a label `test-release` one triggers the release workflow publishing to test pypi.
rework doc
e615cf6 to
5da21bb
Compare
By pushing a tag of the form
test-v<VERSION_IDENTIFIERone can trigger the release workflow to publish on test.pypi. This is intended to be used on a release before it gets merged.Here an example run of how the environment is set depending on the tag, see the "Did it work?" task https://github.com/agoscinski/aiida-core/actions/runs/15189282862/job/42717934704
Anyone unhappy on the way we trigger it? I feel like completely automatizing it (like if the PR name starts with "Release") will end up with unintended or flaky behavior so still requiring to manually trigger is the stable way.