Add workflow_run api + webhook #33964
Open
+2,801
−217
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
GiteaBot
added
the
lgtm/need 2
github-actions
bot
added
modifies/translation
modifies/api
|
Verification update of keda k8s autoscaler (current gitea 1.24-rc works only with modified garm). Uses these apis
I could change the existing keda github-runner e2e test to pass on my end by manually running gitea and setting up the test repository without changing non test content. Maybe we could setup extended e2e test repositories to track external autoscaler compatibility |
4 tasks
This was referenced May 7, 2025
This comment was marked as outdated.
This comment was marked as outdated.
…stopherHX/33964-1
…stopherHX/33964-1
…stopherHX/33964-1
|
Ready for review? |
|
My time to respond or make changes is limited in the next seven days, due to exams. Other than that, yes I think this is ready for review. |
wxiaoguang
reviewed
Jun 19, 2025
| ListOptions: utils.GetListOptions(ctx), | ||
| } | ||
|
|
||
| if event := ctx.Req.URL.Query().Get("event"); event != "" { |
There was a problem hiding this comment.
URL.Query will do ParseQuery again and agian.
Maybe we can just call FormString, or use a parsed query parsedQuery := ctx.Req.URL.Query()
wxiaoguang
reviewed
Jun 19, 2025
|
|
||
| runID := ctx.PathParamInt64("run") | ||
|
|
||
| shared.ListJobs(ctx, 0, repoID, runID) |
There was a problem hiding this comment.
Maybe ListJobs has done enough check to ensure runID belongs to repoID, while is it possible to explicitly check it here, or add comment to ListJobs to explain that no need extra check?
wxiaoguang
reviewed
Jun 19, 2025
| // Sync run status with db | ||
| job.Run = nil | ||
| if err := job.LoadAttributes(ctx); err != nil { | ||
| ctx.HTTPError(http.StatusInternalServerError, err.Error()) |
There was a problem hiding this comment.
use ctx to report server internal error, then the err will also appear in logs
wxiaoguang
reviewed
Jun 19, 2025
Comment on lines
+90
to
+95
| // Sync run status with db | ||
| jobs[0].Run = nil | ||
| if err := jobs[0].LoadAttributes(ctx); err != nil { | ||
| return err | ||
| } | ||
| run := jobs[0].Run |
There was a problem hiding this comment.
Saw this code block many times. Maybe a common function:
func notifyWorkflowRunStatusUpdate(....) {
job.Run = nil
if err := job.LoadAttribute(); err != nil {
log.Error()
} else {
notify_service.WorkflowRunStatusUpdate(
}
}
wxiaoguang
reviewed
Jun 19, 2025
|
|
||
| status := convert.ToWorkflowRunAction(run.Status) | ||
|
|
||
| gitRepo, err := gitrepo.OpenRepository(context.Background(), repo) |
wxiaoguang
approved these changes
Jun 19, 2025
GiteaBot
added
lgtm/need 1
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Implements
/actions/runsfor global + user + org (Gitea only)/actions/jobsfor global + user + org + repository (Gitea only)Fixes
runs/<run>part to align with apiRefactoring
TODO
Closes #23670
Closes #23796
Closes #24898
Replaces #28047 and is much more complete