-
-
Notifications
You must be signed in to change notification settings - Fork 6.1k
Cleanup ActionRun creation #35624
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?
Cleanup ActionRun creation #35624
Changes from all commits
678a970
0aec839
948f650
0cc6d31
a5aea1b
a00cff0
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 |
---|---|---|
|
@@ -9,15 +9,79 @@ import ( | |
|
||
actions_model "code.gitea.io/gitea/models/actions" | ||
"code.gitea.io/gitea/models/db" | ||
"code.gitea.io/gitea/modules/log" | ||
"code.gitea.io/gitea/modules/util" | ||
notify_service "code.gitea.io/gitea/services/notify" | ||
|
||
"github.com/nektos/act/pkg/jobparser" | ||
"gopkg.in/yaml.v3" | ||
) | ||
|
||
// PrepareRunAndInsert prepares a run and inserts it into the database | ||
// It parses the workflow content, evaluates concurrency if needed, and inserts the run and its jobs into the database. | ||
// The title will be cut off at 255 characters if it's longer than 255 characters. | ||
func PrepareRunAndInsert(ctx context.Context, content []byte, run *actions_model.ActionRun, inputsWithDefaults map[string]any) error { | ||
if err := run.LoadAttributes(ctx); err != nil { | ||
return fmt.Errorf("LoadAttributes: %w", err) | ||
} | ||
|
||
vars, err := actions_model.GetVariablesOfRun(ctx, run) | ||
if err != nil { | ||
return fmt.Errorf("GetVariablesOfRun: %w", err) | ||
} | ||
|
||
wfRawConcurrency, err := jobparser.ReadWorkflowRawConcurrency(content) | ||
if err != nil { | ||
return fmt.Errorf("ReadWorkflowRawConcurrency: %w", err) | ||
} | ||
|
||
if wfRawConcurrency != nil { | ||
err = EvaluateRunConcurrencyFillModel(ctx, run, wfRawConcurrency, vars) | ||
if err != nil { | ||
return fmt.Errorf("EvaluateRunConcurrencyFillModel: %w", err) | ||
} | ||
} | ||
|
||
giteaCtx := GenerateGiteaContext(run, nil) | ||
|
||
jobs, err := jobparser.Parse(content, jobparser.WithVars(vars), jobparser.WithGitContext(giteaCtx.ToGitHubContext()), jobparser.WithInputs(inputsWithDefaults)) | ||
if err != nil { | ||
return fmt.Errorf("parse workflow: %w", err) | ||
} | ||
|
||
if len(jobs) > 0 && jobs[0].RunName != "" { | ||
run.Title = jobs[0].RunName | ||
} | ||
|
||
if err := InsertRun(ctx, run, jobs, vars); err != nil { | ||
return fmt.Errorf("InsertRun: %w", err) | ||
} | ||
|
||
// FIXME PERF do we need this db round trip? | ||
allJobs, err := db.Find[actions_model.ActionRunJob](ctx, actions_model.FindRunJobOptions{RunID: run.ID}) | ||
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. I wonder why we do not use job objects that insertrun creates? This db read seems like to not bring any advantage for me. |
||
if err != nil { | ||
log.Error("FindRunJobs: %v", err) | ||
} | ||
|
||
// FIXME PERF skip this for schedule, dispatch etc. | ||
CreateCommitStatus(ctx, allJobs...) | ||
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. old code conditionally didn't call this, nested function does the check |
||
|
||
err = run.LoadAttributes(ctx) | ||
if err != nil { | ||
log.Error("LoadAttributes: %v", err) | ||
} | ||
notify_service.WorkflowRunStatusUpdate(ctx, run.Repo, run.TriggerUser, run) | ||
for _, job := range allJobs { | ||
notify_service.WorkflowJobStatusUpdate(ctx, run.Repo, run.TriggerUser, job, nil) | ||
} | ||
|
||
// Return nil if no errors occurred | ||
return nil | ||
} | ||
|
||
// InsertRun inserts a run | ||
// The title will be cut off at 255 characters if it's longer than 255 characters. | ||
func InsertRun(ctx context.Context, run *actions_model.ActionRun, jobs []*jobparser.SingleWorkflow) error { | ||
func InsertRun(ctx context.Context, run *actions_model.ActionRun, jobs []*jobparser.SingleWorkflow, vars map[string]string) error { | ||
return db.WithTx(ctx, func(ctx context.Context) error { | ||
index, err := db.GetNextResourceIndex(ctx, "action_run_index", run.RepoID) | ||
if err != nil { | ||
|
@@ -44,12 +108,6 @@ func InsertRun(ctx context.Context, run *actions_model.ActionRun, jobs []*jobpar | |
return err | ||
} | ||
|
||
// query vars for evaluating job concurrency groups | ||
vars, err := actions_model.GetVariablesOfRun(ctx, run) | ||
if err != nil { | ||
return fmt.Errorf("get run %d variables: %w", run.ID, err) | ||
} | ||
|
||
runJobs := make([]*actions_model.ActionRunJob, 0, len(jobs)) | ||
var hasWaitingJobs bool | ||
for _, v := range jobs { | ||
|
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.
Can we use this
vars
inInsertRun
to avoid querying vars again on services/actions/run.go L112?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.
Sounds good to me to add
vars
as parameter toInsertRun
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.
Passing this as parameter to InsertRun in a5aea1b
I think that caching vars context is good to be part of an interface that is part of the passed context of the workflow run.