utils: Allow execute steps to contribute success or fail items for display in the activity log #1766
Conversation
This reverts commit 5852321.
| if (!step.shouldExecute(this._context)) { | ||
| step = steps.pop(); | ||
| continue; | ||
| } | ||
|
|
||
| let output: types.ExecuteActivityOutput | undefined; | ||
| try { | ||
| this._context.telemetry.properties.lastStep = `execute-${getEffectiveStepId(step)}`; | ||
| await step.execute(this._context, internalProgress); | ||
| output = step.createSuccessOutput?.(this._context); | ||
| } catch (e) { | ||
| output = step.createFailOutput?.(this._context); | ||
| if (!step.options.continueOnFail) { | ||
| throw e; | ||
| } | ||
| } finally { | ||
| output ??= {}; | ||
| this.displayActivityOutput(output, step.options); | ||
|
|
||
| currentStep += 1; | ||
| step = steps.pop(); | ||
| } |
There was a problem hiding this comment.
Could you explain the changes made here a bit? I just want to make sure we understand them fully before making changes to the wizard since it's so central to so many commands.
There was a problem hiding this comment.
Yeah sure, so basically all execute steps can now have the following optional methods: createFailOutput & createSuccessOutput. The output return signature for these methods is basically just an object with two properties: message?: string and item?: AzExtTreeItem (the tree item will be pushed in as an activity child). These outputs indicate what needs to be displayed to the user upon successful or unsuccessful execution of an execute step. If these optional methods are not defined on the step, then there should be no real difference to how our wizard steps currently run.
Success flow:
- In the try block, assuming the step executes without throwing an error
- (Line 199) We try to get an output from the step (optional since the method may not exist). Output will either be the defined success output or
undefinedif not set. - (Line 207) If initial return was
undefined, output will be an empty object anddisplayActivityOutputwill display nothing to the user. - (Line 207) If defined,
displayActivityOutputwill display any defined output message to the output log, and push the tree item into theactivityChildrenarray if one exists on the context. If any options to suppress showing are set on the execute step, these will be honored here.
Fail flow:
- In the try block, assuming the step executes and throws an error
- (Line 201) We try to get an output from the step (optional since the method may not exist). Output will be either the defined fail output or
undefinedif not set. - (Line 202) New step option allows for us to essentially swallow errors. In some cases, we don't necessarily want the entire wizard to stop just because a step failed. So when a step has
continueOnFailset, don't throw the error, but still display any defined output unless it has been explicitly set to be suppressed in the step options. - (Line 207) If initial value was
undefined, output will be an empty object anddisplayActivityOutputwill display nothing to the user. - (Line 207) If defined,
displayActivityOutputwill display any defined output message to the output log, and push any tree item into theactivityChildrenarray if it has been initialized. If any options to suppress showing are set on the execute step, these will be honored here.
Rest of the code should be basically the same. I did use a guard clause with the shouldExecute to try and remove some nesting since we were adding more try/catch blocks. Let me know if that makes sense / if you have any questions / suggestions!
This implementation will enable execute steps to contribute success or fail items (as activity children) to the activity log upon execution. The resulting
ExecuteActivityOutputwill be handled by the Azure Wizard which has been updated accordingly.Example of what this would look like in the activity log:
Example with
continueOnFailset to true when a step fails:Example of how it will probably look implemented in ACA:
microsoft/vscode-azurecontainerapps#722