Skip to content
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

Delete run feature #26275

Closed
wants to merge 4 commits into from
Closed

Conversation

cassiozareck
Copy link
Contributor

@cassiozareck cassiozareck commented Aug 1, 2023

Target Issue: #26219

This PR has the aim of add a delete single workflow run feature to Gitea. It does that by:

  1. Adding in web.go a new delete endpoint for the action that calls action.Delete.
  2. In the Delete function it gets the respective ActionRun and ActionRunJob objects by run ID, starts a transaction and calls 2 lower-level functions at models/actions/run.go: DeleteRun and DeleteRunJobs.
  3. These lower-level functions will be responsible to do the DB operations to delete it.

This work is still in progress and I'm aware it needs more operations to really erase every remnant of the deleted Run. Also it needs to be integrated with the frontend.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Aug 1, 2023
@pull-request-size pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Aug 1, 2023
models/actions/run.go Outdated Show resolved Hide resolved
@cassiozareck
Copy link
Contributor Author

cassiozareck commented Aug 3, 2023

Hey @KN4CK3R thanks for helping me. I want to ask you a few question:

1- Do we need to decrease the action_run_index? If we dont then when inserting new run actions ids will not be linear if we deleted some Run. Like so:
Screenshot from 2023-08-03 11-51-08
I also want to cover this but seems hard to decrease the NextResourceIndex

2- When inserting new runs there's a TaskVersion that is increased. I dont know exactly its purposes by now either if we need to do something with it when deleting.

Overall the functionality is working to delete run workflows as well its jobs. By now I'll also implement a way of delete stored jobs tasks that are associate with a non-existent job ID.

@KN4CK3R
Copy link
Member

KN4CK3R commented Aug 3, 2023

1- Do we need to decrease the action_run_index? If we dont then when inserting new run actions ids will not be linear if we deleted some Run. Like so: Screenshot from 2023-08-03 11-51-08 I also want to cover this but seems hard to decrease the NextResourceIndex

I see no reason to do this. The id should be unique. If you delete an entry there should be no way to reuse the old id.

@pull-request-size pull-request-size bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 5, 2023
@cassiozareck
Copy link
Contributor Author

cassiozareck commented Aug 5, 2023

I've updated the PR now its also removing ActionTaskSteps. The only thing that remains is to also remove the ActionTask... I've spend some time in it but I couldn't figure out why it isn't being removed from database. It reaches until run.DeleteTask correctly with the correct ID. But the Row just stays at the DB even after the DeleteBeans has executed. There isn't any error. Everything else is deleted correctly.
I'll ping @wolfogre as he made most of the Actions logic maybe he can help me there also.

@pull-request-size pull-request-size bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Aug 6, 2023
@wolfogre
Copy link
Member

wolfogre commented Aug 7, 2023

But the Row just stays at the DB even after the DeleteBeans has executed. There isn't any error. Everything else is deleted correctly.

I haven't looked at the code yet, but I think it may be an incorrect use of xorm, you could enable SQL log to debug it.

I'll ping @wolfogre as he made most of the Actions logic maybe he can help me there also.

Some logic which could help:

  • What you want delete is a run, so you need to delete a row of ActionRun.
  • A run could include multiple jobs, so you need to delete multiple rows of ActionRunJob.
  • When a job has been picked up by a runner to execute, there will be a task, however, a job could be rerun many times, so you need to delete multiple rows of ActionTask.
  • Every task could include
    • multiple steps, so ActionTaskStep
    • multiple outputs, so ActionTaskOutput
  • The log of the task is stored as a file (in DBFS or storage), so you need to detele it according to ActionTask.LogFilename or ActionTask.LogInStorage.
  • In addition, there may be some artifacts uploaded to the run, so you need to delete multiple rows of ActionArtifact.
    • The files of artifacts are stored in storage, so you need to remove them.
  • Finally, update NumActionRuns and NumClosedActionRuns of Repository.

To be honest, I don't think it's a small job. See also #24256

@go-gitea go-gitea locked as resolved and limited conversation to collaborators Jan 31, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants