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

Set state_output_profile as option and update docs #59165

Merged
merged 7 commits into from
Feb 23, 2021
Merged

Set state_output_profile as option and update docs #59165

merged 7 commits into from
Feb 23, 2021

Conversation

ScriptAutomate
Copy link
Contributor

What does this PR do?

  • Documents state_output_profile as an option in master and minion configs
  • Adds state_output_profile as an option to salt/config/__init__.py, validating as a bool
  • Adds state_output_profile as having default value of True in minion and master configs
  • Updates salt/output/highstate.py to no longer hard-code default value for state_output_profile

What issues does this PR fix or reference?

Ongoing work related to: #58112

It looks like state_output_profile was meant to be an option, but was instead hard-coded into salt/output/highstate.py when the value and option should have been documented for minion/master configurations.

Previous Behavior

Undocumented option.

New Behavior

Documented option.

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

Commits signed with GPG?

Yes

@ScriptAutomate ScriptAutomate requested a review from a team as a code owner December 18, 2020 03:56
@ScriptAutomate ScriptAutomate requested review from waynew and removed request for a team December 18, 2020 03:56
@ScriptAutomate
Copy link
Contributor Author

Of note: I also opened #59166 after noticing that we may need to extend the salt/output/highstate.py docstring documentation to include state_output_diff and state_output_profile information. The issue also notes that show_output documentation in the highstate.py docstring docs differs from the master/minion config definition of what valid values are acceptable.

Copy link
Contributor

@waynew waynew left a comment

Choose a reason for hiding this comment

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

🚀 Looks great!

Copy link
Contributor

@waynew waynew left a comment

Choose a reason for hiding this comment

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

Looks like in JsonNestedTestCase in the setup_loader_modules function you'll need to add "state_output_profile": True, as well

@ScriptAutomate
Copy link
Contributor Author

ScriptAutomate commented Jan 19, 2021

@waynew said: Looks like in JsonNestedTestCase in the setup_loader_modules function you'll need to add "state_output_profile": True, as well

Thanks!

I've now changed here:

pre-commit applied everything else here:

@ScriptAutomate ScriptAutomate requested a review from waynew January 19, 2021 22:15
Copy link
Contributor

@waynew waynew left a comment

Choose a reason for hiding this comment

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

🚀

@ScriptAutomate ScriptAutomate added the Aluminium Release Post Mg and Pre Si label Feb 23, 2021
@Ch3LL Ch3LL merged commit 1ba6a5d into saltstack:master Feb 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Aluminium Release Post Mg and Pre Si
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants