Support Conditional Steps in Composite Actions #1438
Conversation
| @@ -1034,7 +1034,6 @@ private ActionSetupInfo PrepareRepositoryActionAsync(IExecutionContext execution | |||
| } | |||
| } | |||
|
|
|||
| // TODO: remove once we remove the DistributedTask.EnableCompositeActions FF | |||
| foreach (var step in compositeAction.Steps) | |||
| { | |||
| if (string.IsNullOrEmpty(executionContext.Global.Variables.Get("DistributedTask.EnableCompositeActions")) && step.Reference.Type != Pipelines.ActionSourceType.Script) | |||
There was a problem hiding this comment.
do we still need the FF in the runner? I guess the answer might be yes for older version GHES.
There was a problem hiding this comment.
I suppose when we do remove the FF we would search the runner for its references, not sure if leaving the TODO made sense.
| @@ -123,6 +123,7 @@ | |||
| "properties": { | |||
| "name": "string-steps-context", | |||
| "id": "non-empty-string", | |||
| "if": "step-if", | |||
There was a problem hiding this comment.
add some L0 tests for this new keyword
There was a problem hiding this comment.
Added L0 to test parsing ${{ inputs.* }} and functions like always(). Let me know if you think we should test something else.
| return jobStatus == ActionResult.Failure; | ||
|
|
||
| // Decide based on 'action_status' for composite MAIN steps and 'job.status' for pre, post and job-level steps | ||
| var isCompositeMainStep = executionContext.IsEmbedded && executionContext.Stage == ActionRunStage.Main; |
There was a problem hiding this comment.
add some L0 for these new functions.
Do we need to change other ExpressionFunctions?
There was a problem hiding this comment.
Other than Success and Failure, we've got
AlwaysFunction: It's a constant return true
CancelledFunction : dependent on jobStatus == ActionResult.Cancelled, so no change required
HashFunction: irrelevant
There was a problem hiding this comment.
Added L0s for Success and Failure
| @@ -241,6 +241,10 @@ private async Task RunStepsAsync(List<IStep> embeddedSteps, ActionRunStage stage | |||
| step.ExecutionContext.ExpressionFunctions.Add(new FunctionInfo<FailureFunction>(PipelineTemplateConstants.Failure, 0, 0)); | |||
| step.ExecutionContext.ExpressionFunctions.Add(new FunctionInfo<SuccessFunction>(PipelineTemplateConstants.Success, 0, 0)); | |||
|
|
|||
| // Set action_status to the success of the current composite action | |||
There was a problem hiding this comment.
are we adding a new github context? action_status
There was a problem hiding this comment.
i linked you the ADR
There was a problem hiding this comment.
we have following action related context:
github.action_repositorygithub.action_refgithub.action_statusgithub.action_path
Looks like in composite actions, they might point to different actions (parent vs. child)
There was a problem hiding this comment.
That's right, it's the behaviour we want. - we decided to add github.action_status in that pattern.
action_status only matters to composite actions. If a composite action calls another composite action, we want their github.action_status values to be independent from each other.
| @@ -295,108 +299,100 @@ private async Task RunStepsAsync(List<IStep> embeddedSteps, ActionRunStage stage | |||
| CancellationTokenRegistration? jobCancelRegister = null; | |||
| try | |||
| { | |||
There was a problem hiding this comment.
these change seems risky, do we have any L0 to cover existing behavior?
There was a problem hiding this comment.
No L0s for *Handler files afaik, but I wrote up a good number of composite action E2Es we could merge into /canary.
|
Woohoo! Thanks everyone 🥳 |
Support has been added in actions/runner#1438 The issue was being tracked in actions/runner#834 The "metadata syntax" documentation has not been updated yet, so the link and copy may need to change.
Support has been added in actions/runner#1438 The issue was being tracked in actions/runner#834 The "metadata syntax" documentation has not been updated yet, so the link and copy may need to change.
* conditional support for composite actions * Fix Conditional function evaluation * Push launch.json temporarily * Revert "Push launch.json temporarily" * rename context * Cleanup comments * fix success/failure functions to run based on pre/main steps * idea of step_status * change to use steps context, WIP * add inputs to possible if condition expressions * use action_status * pr cleanup * Added right stages * Test on stage in conditional functions * Fix naming and formatting * Fix tests * Add success and failure L0s * Remove comment * Remove whitespace * Undo formatting * Add L0 for step-if parsing * Add ADR Co-authored-by: Thomas Boop <[email protected]>
No description provided.