Skip to content
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

refactor(stages): Fixed alias matching, added fallback and unit tests #7080

Merged
merged 7 commits into from
Jun 17, 2019

Conversation

alanmquach
Copy link
Contributor

Experimental: This would allow multiple implementations of a single stage type. Similar to how a specific type can be provided by multiple cloud providers, but in this case we use a generic altKey instead of cloudProvider for matching.

@anotherchrisberry
Copy link
Contributor

Does this buy us anything over using alias?

@alanmquach
Copy link
Contributor Author

I was thinking this would have absolutely 0 dependency on anything in orca, but let me take a look and play with alias a bit more.

@alanmquach alanmquach changed the title refactor(stages): Support altKey lookups in the pipeline registry refactor(stages): Fixed alias matching, added fallback and unit tests Jun 13, 2019
@alanmquach
Copy link
Contributor Author

Turns out alias almost works like a charm.

I ended up only having to refactor PipelineRegistry.getStageConfig() a bit to fix the corner case where aliasing to an existing key would break the existing stage. For example, simply registering a new { key: 'foo', alias: 'wait' } stage would break wait because:

  • getStageConfig returns null
  • this is because the existence of an extra alias increases the number of matches (since the existing filter included stageType.alias === stage.type)
  • this ends up bumping typical things from 1 match to 2 (or more), causing it to go into the default block of the switch statement (which only handles finding a cloudProvider specific stage type)
  • stages that don't have cloud providers would simply return null

This is basically what the new unit test matches redirect targets to ensure the actual stages do not get broken simply by having other stages alias to them will now protect against.

Finally, also added the ability to gracefully degrade back to the aliased stage (wait in the above example) if aliasing stage (foo) is ever deleted/deprecated from deck.

Copy link
Contributor

@anotherchrisberry anotherchrisberry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice

@alanmquach alanmquach merged commit c88234c into spinnaker:master Jun 17, 2019
alanmquach added a commit to alanmquach/deck that referenced this pull request Jun 18, 2019
939d608 fix(pipeline): fix invisible parameter when default is not in options (spinnaker#7125)
5c4facb fix(core/pipeline): stop searching stage context, being greedy about parentExecutions (spinnaker#7127)
8fe74bc fix(core): do not inject default execution window values on render (spinnaker#7122)
ee6fbe0 fix(core/pipeline): use correct visibility default for stage durations (spinnaker#7121)
c88234c refactor(stages): Fixed alias matching, added fallback and unit tests (spinnaker#7080)
c358801 refactor(core): Reactify ExecutionWindows component (spinnaker#7113)
744123d fix(artifact): use artifact icons in server group link (spinnaker#7118)
b8aebd7 fix(forms): Fixed SpelText not firing onChange upon autocomplete (spinnaker#7114)
4e2f749 refactor(core): reactify overrideFailure component (spinnaker#7107)
4f07aeb fix(core): Make template table list scrollable (spinnaker#7111)
e5d6115 fix(core): Display template inherited items (mptv2) as read only (spinnaker#7102)
anotherchrisberry pushed a commit that referenced this pull request Jun 18, 2019
939d608 fix(pipeline): fix invisible parameter when default is not in options (#7125)
5c4facb fix(core/pipeline): stop searching stage context, being greedy about parentExecutions (#7127)
8fe74bc fix(core): do not inject default execution window values on render (#7122)
ee6fbe0 fix(core/pipeline): use correct visibility default for stage durations (#7121)
c88234c refactor(stages): Fixed alias matching, added fallback and unit tests (#7080)
c358801 refactor(core): Reactify ExecutionWindows component (#7113)
744123d fix(artifact): use artifact icons in server group link (#7118)
b8aebd7 fix(forms): Fixed SpelText not firing onChange upon autocomplete (#7114)
4e2f749 refactor(core): reactify overrideFailure component (#7107)
4f07aeb fix(core): Make template table list scrollable (#7111)
e5d6115 fix(core): Display template inherited items (mptv2) as read only (#7102)
@alanmquach alanmquach deleted the alt-key-stages branch June 27, 2019 04:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants