Skip to content

Fix and consolidate generation of st2.conf.sample #5710

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

Merged
merged 4 commits into from
Aug 19, 2022
Merged

Conversation

cognifloyd
Copy link
Member

Background

I'm working towards introducing pants. One of the first things we will use it for is linting / reformatting (it will run black, flake8, etc). Updating sample conf must happen whenever the source python files change. Pants will know which file changes require regenerating the sample conf, so it is advantageous to let it handle that. So, we'll hook that up so pants runs it when it runs fmt and other tools.

Overview

Pants will not use the Makefile to generate st2.conf.sample, it will use tools/config_gen.py directly. So, this moves a header comment from the Makefile into tools/config_gen.py, with the rest of the generation logic.

It also fixes a long standing issue where our st2.conf.sample file was actually invalid. [sensorcontainer].partition_provider should not have a python dict literal, but that's how it got serialized. It should have space separted key:value pairs for partition_provider config. So, this also adjust tools/config_gen.py to fix that issue.

@cognifloyd cognifloyd added this to the 3.8.0 milestone Aug 12, 2022
@cognifloyd cognifloyd self-assigned this Aug 12, 2022
@pull-request-size pull-request-size bot added the size/S PR that changes 10-29 lines. Very easy to review. label Aug 12, 2022
@cognifloyd cognifloyd requested review from nzlosh, rush-skills and a team August 12, 2022 20:25
Copy link
Member

@arm4b arm4b left a comment

Choose a reason for hiding this comment

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

Thanks for the fix, good catch! 👍

Comment on lines +80 to +83
INIT_COMMENT = """
# Sample config which contains all the available options which the corresponding descriptions
# Note: This file is automatically generated using tools/config_gen.py - DO NOT UPDATE MANUALLY
""".strip()
Copy link
Member

Choose a reason for hiding this comment

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

that's definitely much better place for the config autogenerated note 👍

@cognifloyd cognifloyd requested a review from a team August 16, 2022 20:47
@cognifloyd cognifloyd enabled auto-merge August 18, 2022 01:46
@cognifloyd cognifloyd requested review from a team and removed request for rush-skills August 19, 2022 14:53
@cognifloyd cognifloyd merged commit 70b7c2e into master Aug 19, 2022
Copy link
Contributor

@nzlosh nzlosh left a comment

Choose a reason for hiding this comment

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

LGTM

@cognifloyd cognifloyd deleted the sample-config-gen branch August 19, 2022 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug fix pantsbuild refactor size/S PR that changes 10-29 lines. Very easy to review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants