Prune conda nightlies on release #11294
Conversation
Unit Test ResultsSee test report for an extended history of previous test failures. This is useful for diagnosing flaky tests. 13 files ± 0 13 suites ±0 3h 3m 21s ⏱️ -55s Results for commit 223b915. ± Comparison against base commit a747fd4. ♻️ This comment has been updated with latest results. |
| @@ -38,6 +54,15 @@ jobs: | |||
| use-mamba: true | |||
| python-version: 3.9 | |||
| channel-priority: strict | |||
| - name: Remove outdated conda nightlies | |||
| if: github.event_name == 'push' && github.repository == 'dask/dask' && startsWith(github.ref, 'refs/tags/') | |||
There was a problem hiding this comment.
Do we want to check is_tagged_commit here as well? Or is that not needed for some reason?
There was a problem hiding this comment.
Or is that not needed for some reason?
At this point we should've already successfully prevented the race condition described in #11294 (comment), so think at this point the value of is_tagged_commit is interchangeable with that of startsWith(github.ref, 'refs/tags/'), such that we could represent this with something like
| if: github.event_name == 'push' && github.repository == 'dask/dask' && startsWith(github.ref, 'refs/tags/') | |
| if: github.event_name == 'push' && github.repository == 'dask/dask' && needs.check_tag.outputs.is_tagged_commit == 'true' |
We should be good with what we have here, not sure if the suggestion would improve readability at all
| @@ -4,7 +4,7 @@ on: | |||
| branches: | |||
| - main | |||
| tags: | |||
| - "*" | |||
| - "*.*.*" | |||
There was a problem hiding this comment.
Am curious why the additional .*s are needed now. Are there some cases we would like to filter out?
There was a problem hiding this comment.
This was just lifted from the on.push.tags block in the release publisher workflow - made this change to be in line with what we consider an event for a pushed "release" tag, but don't think it should be needed for the rest of these changes and can drop if that makes sense
| needs: check_tag | ||
| if: startsWith(github.ref, 'refs/tags/') || needs.check_tag.outputs.is_tagged_commit == 'false' |
There was a problem hiding this comment.
So this needs to be a tag or not? Is there a particular case we are trying to exclude here?
There was a problem hiding this comment.
TLDR - this filters out branch push-triggered runs if we see the push was associated with a tag, since we already know this workflow was triggered by the corresponding tag push
Since #11014 there's been a race condition on release (which coincides with the push of a tagged commit) where this workflow is triggered twice for a branch and tag push and attempts to upload the same package twice. Consider these workflows for the 2024.8.1 release, one passing and one failing at the upload stage:
- https://github.com/dask/dask/actions/runs/10426906155/job/28880711510
- https://github.com/dask/dask/actions/runs/10426906162/job/28880711395
There are probably a few different ways to handle something like this, but I opted to check if the current HEAD is an exact match with a tag as a filtering condition to stop branch-triggered runs since at that point we know there's a corresponding tag triggered run, which we can now treat as the "definitive" run of this workflow on a release.
| # install anaconda for removal | ||
| mamba install -c conda-forge anaconda-client | ||
|
|
||
| anaconda remove dask/dask-core --force |
There was a problem hiding this comment.
Do we want to make sure the nightlies removed are older than a certain version? Or is that case unlikely to occur?
There was a problem hiding this comment.
Definitely think if we wanted to, we could be explicit here and specify to remove all packages older than the current computed "nightly" version rather than everything, which could guard against the edge case that we somehow upload post-release nighties to the c 8000 hannel before the release workflow run is able to reach this point.
Were you thinking of any particular edge cases? In my head, I see the above case as niche enough that we'd probably not encounter even if we merged this in as is, but want to make sure I'm not missing a more crucial situation here
|
Rerun tests |
Contributes to dask/community#394
This PR makes the following changes to our conda nightly workflow:
anaconda remove dask/dask-coreto prune out all existing nightlies prior to uploading a new onecheck_tagjob, which is used to conditionally skip building/uploading on pushes tomainthat are tagged, since at this point the same workflow should've already been triggered by the tag push itself