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

Add support for --load-yml to cci task and cci flow commands #3725

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

jlantz
Copy link
Contributor

@jlantz jlantz commented Jan 2, 2024

The BaseCumulusCI runtime has long had support for passing additional_yaml which is then added to the yaml merge stack. This functionality was used extensively in MetaCI to inject build specific configuration overrides for certain build plans. However, it was never exposed to the cci cli.

This PR adds a new --load-yml <path/to/additional.yml> option flag for all subcommands of cci task and cci flow by adding support for the option in the main command and then implementing it as an option for the cci task and cci flow commands.

This change should have no impact on existing users and is purely additive functionality that didn't exist before. I've added a docs section to the config docs explaining the use case which is included with this PR along with full test coverage.

@jlantz jlantz requested a review from a team as a code owner January 2, 2024 01:42
@@ -0,0 +1,14 @@
flows:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps this belongs alongside the tests instead of here?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I suggest adding it inline in the tests and using tmp_path to write the contents to disk during the test. If that's not workable, then moving it to the same subdirectory as the tests is fine.

Copy link
Contributor

@jstvz jstvz left a comment

Choose a reason for hiding this comment

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

Thanks @jlantz. Two tweaks needed here.

@@ -0,0 +1,14 @@
flows:
Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I suggest adding it inline in the tests and using tmp_path to write the contents to disk during the test. If that's not workable, then moving it to the same subdirectory as the tests is fine.


## Loading additional yaml configuration from a file

CumulusCI supports loading in an additional yaml file from the command line with the `--load-yml <path/to/file>` option on `cci task` and `cci flow` commands. This can be useful if you have one-off automation configurations that you want to keep out of the main project's configuration and load only in special cases.
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for including documentation with your PR. I think that there should be a mention of this option in the configuration scopes section above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the review. I'll get these addressed shortly.

@jstvz
Copy link
Contributor

jstvz commented Jun 20, 2024

Fixes #3429.

@jstvz
Copy link
Contributor

jstvz commented Sep 24, 2024

@jlantz Following up on this PR.

@jlantz
Copy link
Contributor Author

jlantz commented Sep 25, 2024

I will get those two changes done and submitted this week, along with a whole bunch of new stuff to put in the review queue ;)

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

Successfully merging this pull request may close these issues.

2 participants