-
Notifications
You must be signed in to change notification settings - Fork 8
Delete old releases #21
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
base: main
Are you sure you want to change the base?
Conversation
…CI build fix is valid." This reverts commit 1bc10af.
|
Hi @marbre, @stellaraccident, @saienduri, @sjain-stanford can you take a look please? Don't have write permissions in this repo, so just tagging you all. Thanks! |
I was made aware of this issue during the LLVM dev mtg but didn't had a chance to yank old releases as I was still traveling. Hoping to find a bit more time soon. |
|
As another user/stakeholder of the python packages -- thank you, @sahas3! Upon discussion with @marbre and @banach-space, we also figured the 1000 files limit is the current issue. The changes you propose seem right on the money to me 👍 |
banach-space
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LG % minor comments/suggestions
Thanks for implementing this!
banach-space
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LG % minor comments/suggestions
Thanks for implementing this!
51890ea to
5d1fa67
Compare
rolfmorel
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Keeping old nightlies around for a bit longer is friendlier for other projects, e.g. in terms of PRs (e.g. on https://github.com/llvm/lighthouse) that are open for a longer period and still have the old nightly in their environment config files. Torch-mlir is looking like it will switch to keeping 60 days of old nightlies: llvm/torch-mlir-release#21 . For eudsl, I come to a limit of roughly 30 days due to every nightly consisting of 30 assets or less (the mlir-python-bindings being at 30 a night). With the purge after 30 days, it should be we don't run afoul of GitHub's 1000 assets-per-release limit.
Keeping old nightlies around for a bit longer is friendlier for other projects, e.g. in terms of PRs (e.g. on https://github.com/llvm/lighthouse) that are open for a longer period and still have the old nightly in their environment config files. Torch-mlir is looking like it will switch to keeping 60 days of old nightlies: llvm/torch-mlir-release#21 . For eudsl, I come to a limit of roughly 30 days due to every nightly consisting of 30 assets or less (the mlir-python-bindings being at 30 a night). With the purge after 30 days, it should be we don't run afoul of GitHub's 1000 assets-per-release limit.
marbre
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Excuse the delayed review. A few comments.
Could eventually also use something like
gh release view dev-wheels --json assets --jq '.assets[].name' | grep rc${DATE} | xargs -L1 gh release delete-asset dev-wheels
7ad3a9d to
14be394
Compare
14be394 to
85aba77
Compare
marbre
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, let's give this a try.
Releasing the wheels has been failing for a month now https://github.com/llvm/torch-mlir-release/actions/runs/19032410905/job/54349329471#step:7:30 due to the GH limit that each release artifact can only have a maximum of 1000 artifacts (some discussion captured here https://github.com/orgs/community/discussions/165616).
This PR has two changes:
An alternative to keeping all the assets will probably be to release the assets with new name daily but seems overkill to me. Open to other suggestions as well.
artifactErrorsFailBuildflag on the release CIs which I think will cause the builds to fail if the upload step fails -- currently the build doesn't fail which is why I think we didn't notice this earlier.