Artifact deletion in actions ui#27172
Conversation
|
@fuxiaohei I noticed you've updated the locales for non-English languages. These will be overwritten during the sync from our translation tool Crowdin. If you'd like to contribute your translations, please visit https://crowdin.com/project/gitea. Please revert the changes done on these files. 🍵 |
63d582a to
5646de0
Compare
|
Overall LGTM after above translation change. |
|
Still waiting on @delvh? |
|
@delvh continue to review? |
| if err := storage.ActionsArtifacts.Delete(artifact.StoragePath); err != nil { | ||
| log.Error("Cannot delete artifact %d: %v", artifact.ID, err) | ||
| continue | ||
| } |
There was a problem hiding this comment.
If the file doesn't exist in the storage (eg: site admin deleted them manually), this function won't function anymore (always "continue", and then will be a deadloop if there are a lot pending but non-existing items)
* giteaofficial/main: (28 commits) Improve TrHTML and add more tests (go-gitea#29228) Convert visibility to number (go-gitea#29226) Implement some action notifier functions (go-gitea#29173) Artifact deletion in actions ui (go-gitea#27172) Update docs for actions variables (go-gitea#29239) Refactor more code in templates (go-gitea#29236) Use "Safe" modifier for manually constructed safe HTML strings in templates (go-gitea#29227) Remove jQuery from the repo release form (go-gitea#29225) Make submit event code work with both jQuery event and native event (go-gitea#29223) Remove jQuery from repo migrate page (go-gitea#29219) Remove unneccesary `initUserAuthLinkAccountView` from "link account" page (go-gitea#29217) Fix labels referencing the wrong ID in the user profile settings (go-gitea#29199) Fix label `for` pointing to a `name` instead of `id` in webhook settings (go-gitea#29209) Load outdated comments when (un)resolving conversation on PR timeline (go-gitea#29203) Fix missing template for follow button in organization (go-gitea#29215) Enable markdownlint `no-trailing-punctuation` and `no-blanks-blockquote` (go-gitea#29214) Remove jQuery from the webhook editor (go-gitea#29211) Remove jQuery from issue reference context popup attach (go-gitea#29216) fix typo (go-gitea#29212) Fix debian InRelease Acquire-By-Hash newline (go-gitea#29204) ...
#27172 (comment) When cleanup artifacts, it removes storage first. If storage is not exist (maybe delete manually), it gets error and continue loop. It makes a dead loop if there are a lot pending but non-existing artifacts. Now it updates db record at first to avoid keep a lot of pending status artifacts.
Add deletion link in runs view page. Fix go-gitea#26315  When click deletion button. It marks this artifact `need-delete`. This artifact would be deleted when actions cleanup cron task.
go-gitea#27172 (comment) When cleanup artifacts, it removes storage first. If storage is not exist (maybe delete manually), it gets error and continue loop. It makes a dead loop if there are a lot pending but non-existing artifacts. Now it updates db record at first to avoid keep a lot of pending status artifacts.
go-gitea/gitea#27172 (comment) When cleanup artifacts, it removes storage first. If storage is not exist (maybe delete manually), it gets error and continue loop. It makes a dead loop if there are a lot pending but non-existing artifacts. Now it updates db record at first to avoid keep a lot of pending status artifacts.
Add deletion link in runs view page. Fix go-gitea#26315  When click deletion button. It marks this artifact `need-delete`. This artifact would be deleted when actions cleanup cron task.
|
Automatically locked because of our CONTRIBUTING guidelines |
| m.Post("/approve", reqRepoActionsWriter, actions.Approve) | ||
| m.Post("/artifacts", actions.ArtifactsView) | ||
| m.Get("/artifacts/{artifact_name}", actions.ArtifactsDownloadView) | ||
| m.Delete("/artifacts/{artifact_name}", actions.ArtifactsDeleteView) |
There was a problem hiding this comment.
Does it require reqRepoActionsWriter permission?
There was a problem hiding this comment.
Yes. The check is in the function ArtifactsDeleteView internal code but not here. It's better to move it out of that function.
There was a problem hiding this comment.
-> chore: fix some trivial problems and TODOs #33473
Add deletion link in runs view page.
Fix #26315
When click deletion button. It marks this artifact
need-delete.This artifact would be deleted when actions cleanup cron task.