Skip to content
This repository has been archived by the owner on Dec 13, 2023. It is now read-only.

feat: Add inputTemplate support to workflow metadata #2438

Merged
merged 2 commits into from
Sep 8, 2021
Merged

feat: Add inputTemplate support to workflow metadata #2438

merged 2 commits into from
Sep 8, 2021

Conversation

NickTomlin
Copy link
Contributor

Pull Request type

  • Bugfix
  • Feature
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Other (please describe):

Changes in this PR

This adds an inputTemplate configuration option to workflow definitions that functions similar to the inputTemplate on task definitions .

My current workaround is to define a jq task to do things like provide default variables but this does so more elegantly and maps to the current task modelling.

Alternatives considered

  • An entirely new field
    • I like the fact that inputTemplate is consistent across tasks and workflows, but a new field could have been used here if we wanted to separate the implementations

Questions

  1. Are there any other places I should add tests? The workflow integration tests seem to be the most logical, but please let me know!
  2. I've verified by running the tests locally, but i'm not sure if there's some additional testing/verification I should do manually.

@apanicker-nflx
Copy link
Collaborator

Are there any other places I should add tests? The workflow integration tests seem to be the most logical, but please let me know!

ParametersUtilsTest and WorkflowAndTaskConfigurationSpec provide good coverage for this change. Thanks for adding the tests.

return inputTemplate;
}

public WorkflowDef setInputTemplate(Map<String, Object> inputTemplate) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

To keep it consistent with the other setters in the class, can this also return void?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do!

@aravindanr aravindanr merged commit 1ecbd7f into Netflix:main Sep 8, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Sep 8, 2021

Unit Test Results

   152 files  ±0     152 suites  ±0   8m 23s ⏱️ ±0s
1 115 tests ±0  1 115 ✔️ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 1ecbd7f. ± Comparison against base commit 1ecbd7f.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants