-
Notifications
You must be signed in to change notification settings - Fork 977
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
Composite Actions UI #578
Merged
Merged
Composite Actions UI #578
Conversation
This file contains 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
…bles => but it doesn't 'stick'
…actions/runner into users/ethanchewy/compositeenv
…actions/runner into users/ethanchewy/compositeenv
…actions/runner into users/ethanchewy/compositeenv
…e and only use ExecutionContext.FileTable + update this table when need be
…actions/runner into users/ethanchewy/compositeFileTable
…com/actions/runner into users/ethanchewy/compositeOutputsStep
…d never be non null
…com/actions/runner into users/ethanchewy/compositeOutputsStep
TingluoHuang
approved these changes
Jul 9, 2020
…b.com/actions/runner into users/ethanchewy/compositeUI
…b.com/actions/runner into users/ethanchewy/compositeUI
TingluoHuang
approved these changes
Jul 9, 2020
ethanchewy
changed the base branch from
users/ethanchewy/compositeOutputsStep
to
master
July 13, 2020 21:26
AdamOlech
pushed a commit
to antmicro/runner
that referenced
this pull request
Jan 28, 2021
* Composite Action Run Steps * Env Flow => Able to get env variables and overwrite current env variables => but it doesn't 'stick' * clean up * Clean up trace messages + add Trace debug in ActionManager * Add debugging message * Optimize runtime of code * Change String to string * Add comma to Composite * Change JobSteps to a List, Change Register Step function name * Add TODO, remove unn. content * Remove unnecessary code * Fix unit tests * Fix env format * Remove comment * Remove TODO message for context * Add verbose trace logs which are only viewable by devs * Initial Start for FileTable stuff * Progress towards passing FileTable or FileID or FileName * Sort usings in Composite Action Handler * Change 0 to location * Update context variables in composite action yaml * Add helpful error message for null steps * Pass fileID to all children token of root action token * Change confusing term context => templateContext, Eliminate _fileTable and only use ExecutionContext.FileTable + update this table when need be * Remove unnessary FileID attribute from CompositeActionExecutionData * Clean up file path for error message * Remove todo * Initial start/framework for output handling * Outline different class vs Handler approach * Remove InitializeScope * Remove InitializeScope * Fix Workflow Step Env overiding Parent Env * First Approach for Attaching ID + Group ID to each Composite Action Step * Add GroupID to the ActionDefinitionData * starting foundation for handling clean up outputs step * Pass outputs data to each composite action step to enable set-output functionality * Create ScopeName for whole composite action. This will enable us to add to the StepsContext[ScopeName] for the composite action which will allow us to use all these outputs in the cleanup step * Hook up composite output step to handler => tmmrw implement composite output handler * Add post composite action step to cleanup outputs => triggers composite output cleanup handler * Fix Outputs Token handling start. Add individual step scope names. * Set up Scope Name and Context Name naming system{ * Figured out how to pass Parent Execution Context to clean up step * Figured out how to pass Parent Execution Context and scope names to clean up step * Add GetOutput function for StepsContext * Generate child scope name correctly if parent scope name is null * Simplify InitializeScope() * Outputs are set correctly and able to get all final outputs in handler * Parse through Action Outputs * Fix null ScopeName + ContextName in CompositeOutputHandler * Shift over handling of Action Outputs to output handler * First attempt to fix null retrievals for output variables * Basic Support for Outputs Done. * Clean up pt.1 * Refactor outputs to avoid using Action Reference * Clean up code * Clean up part 2 * Add clarifying comments for the output handler * Remove TODO * Remove env in composite action scope * Clean up * Revert back * revert back * add back envToken * Remove unnecessary code * Add file length check * Clean up * Fix logging issue * Figure out how to handle set-env edge cases * formatting * fix unit tests * Fix windows unit test syntax error * Fix period * Sanity check for fileTable add + remove unn. code * revert back * Add back line break * Fix null errors * Address situation if FileTable is null + add sanity check for adding file to fileTable * add line * Revert * Fix unit tests to instantiate a FileTable * Fix logic for trimming manifestfile path * Add null check * Revert * Revert * revert * spacing * Add filetable to testing file, remove ? since we know filetable should never be non null * Fix Throw logic * Clarify template outputs token * Add another type support for outputs to avoid container unit tests errors * Add mapping for parity * Build support for new outputs format * Build support for new outputs format * Refactor to avoid duplication of action yaml for workflow yaml * revert * revert * revert * spacing
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.
In this PR, we build in support for displaying all the composite action steps into one node.
Demo: https://github.com/ethanchewy/testing-actions/runs/846855088?check_suite_focus=true
Food for thought:
Ok, so I finished the UI Display of the Composite Actions: #578
@TingluoHuang and I realized that the current implementation of composite actions adds steps to the stepRunner. This creates an issue for when we have nested composite steps. For example, right now, the Start and CompleteStep functions are run for each composite step so that keeps on changing the state of the whole composite action to oscillate between "in progress" to "completed" in the UI. We may want to create a separate stepsRunner for just composite actions as well as a separate execution context for composite actions since we need to build a layer of abstraction to eventually support nested composite steps. Since this requires major changes, we will create another PR for this.
tl;dr we probably need to build another abstraction layer for composite actions which could take some time but is important for supporting conditionals and for supporting nested composite actions
This introduces an interesting dilemma for when we support nested composite actions. We will need to explore how we are going to recognize composite actions within composite actions, etc. and how to reconcile the UI.