-
-
Notifications
You must be signed in to change notification settings - Fork 5.8k
Artifact deletion in actions ui #27172
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
Changes from 6 commits
72ac7a0
fa96647
5646de0
cdaacaa
f6843b2
4901bc7
2161da6
4013e2b
6ae3299
0f9196a
aa1e8b6
e5d42a3
56ba94d
ace7a73
fa06309
7032e6d
fceea7f
4fff2c1
925197e
171ff63
650bacb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1328,6 +1328,7 @@ func registerRoutes(m *web.Route) { | |
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does it require There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. The check is in the function There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. -> chore: fix some trivial problems and TODOs #33473 |
||
m.Post("/rerun", reqRepoActionsWriter, actions.Rerun) | ||
}) | ||
}, reqRepoActionsReader, actions.MustEnableActions) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,8 +20,15 @@ func Cleanup(taskCtx context.Context, olderThan time.Duration) error { | |
return CleanupArtifacts(taskCtx) | ||
} | ||
|
||
// CleanupArtifacts removes expired artifacts and set records expired status | ||
// CleanupArtifacts removes expired add need-deleted artifacts and set records expired status | ||
func CleanupArtifacts(taskCtx context.Context) error { | ||
if err := cleanExpiredArtifacts(taskCtx); err != nil { | ||
return err | ||
} | ||
return cleanNeedDeleteArtifacts(taskCtx) | ||
} | ||
|
||
func cleanExpiredArtifacts(taskCtx context.Context) error { | ||
artifacts, err := actions.ListNeedExpiredArtifacts(taskCtx) | ||
if err != nil { | ||
return err | ||
|
@@ -40,3 +47,32 @@ func CleanupArtifacts(taskCtx context.Context) error { | |
} | ||
return nil | ||
} | ||
|
||
// deleteArtifactBatchSize is the batch size of deleting artifacts | ||
const deleteArtifactBatchSize = 100 | ||
|
||
func cleanNeedDeleteArtifacts(taskCtx context.Context) error { | ||
for { | ||
artifacts, err := actions.ListNeedDeleteArtifacts(taskCtx, deleteArtifactBatchSize) | ||
if err != nil { | ||
return err | ||
} | ||
log.Info("Found %d need-deleted artifacts", len(artifacts)) | ||
fuxiaohei marked this conversation as resolved.
Show resolved
Hide resolved
|
||
for _, artifact := range artifacts { | ||
if err := storage.ActionsArtifacts.Delete(artifact.StoragePath); err != nil { | ||
log.Error("Cannot delete artifact %d: %v", artifact.ID, err) | ||
continue | ||
} | ||
Comment on lines
+62
to
+65
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. -> Skip deletion error #33476 |
||
if err := actions.SetArtifactDeleted(taskCtx, artifact.ID); err != nil { | ||
log.Error("Cannot set artifact %d deleted: %v", artifact.ID, err) | ||
continue | ||
} | ||
log.Info("Artifact %d set deleted", artifact.ID) | ||
} | ||
if len(artifacts) < deleteArtifactBatchSize { | ||
log.Debug("No more need-deleted artifacts") | ||
break | ||
} | ||
} | ||
return nil | ||
} |
Uh oh!
There was an error while loading. Please reload this page.