Synchronize st2.conf with upstream #33
Conversation
arm4b
changed the title
You can't overwrite conf_root from `/etc/st2` to anything else, since it's globally hardcoded setting
Convention we started to follow quite a while for all config settings where possible, see: https://github.com/StackStorm/st2/blob/master/conf/st2.package.conf
| @@ -25,5 +25,4 @@ | |||
| hostname 'localhost' | |||
| comment 'Appended by st2 cookbook' | |||
| action :append | |||
There was a problem hiding this comment.
Not sure about that entry if we need it at all. At least after using 127.0.0.1 instead of localhost this rule is out of scope of this cookbook.
From one point of view cleanup might be good, from another side: 127.0.0.1 localhost in hosts won't hurt.
Ideas?
| api_url: 'http://localhost:9101', | ||
| conf_root: '/etc', | ||
| mask_secrets: true, | ||
| allow_origin: '*', |
There was a problem hiding this comment.
Thinking about renaming these config settings:
allow_origin => api_allow_origin
mask_secrets => api_mask_secrets
How it should be, according to: https://github.com/StackStorm/st2/blob/master/conf/st2.package.conf#L10
Let me know your opinions.
There was a problem hiding this comment.
api_ prepending makes more sense
|
BTW, related to this is to support all But it's not a thing for the nearest future. |
|
@jvrplmlmn @sysbot @shortdudey123 Please take a look and let's unblock this Cookbook. Also, we got some more resources and would like to improve this repo. I documented More Features recently. If there is something you would like to see here in future - create an Issue/Document it. And thank you for your involvement! |
| @@ -120,31 +134,34 @@ | |||
| mode: 0644, | |||
| source: 'st2.conf.erb', | |||
| variables: { | |||
There was a problem hiding this comment.
we should alphabetize this
There was a problem hiding this comment.
I'm using it in a way as it appears in st2.conf for the reasons to verify synchronization easier in the future:
https://github.com/StackStorm/st2/blob/master/conf/st2.dev.conf
There was a problem hiding this comment.
If so, you should add a comment about that since whoever modifies this list next will just add it to the end since its not alphabetized
| [sensorcontainer] | ||
| logging = <%= @conf_root %>/st2reactor/console.conf | ||
| logging = /etc/st2/logging.sensorcontainer.conf |
There was a problem hiding this comment.
why remove config option?
There was a problem hiding this comment.
Yeah, included explanation in Commit message:
You can't overwrite
conf_rootfrom/etc/st2to anything else, since it's globally hardcoded setting.
So /etc/st2 is hardcoded in packages, you can't change it and it's used in so many places.
Allowing users to edit conf_root brings confusion and broken installation.
There was a problem hiding this comment.
It's outdated bit that is still here from the legacy installer, before introducing actual .deb/.rpm packages.
I'm going to touch more abandoned things like that in upcoming Cookbook Cleanup.
| logging = <%= @conf_root %>/st2api/console.conf | ||
| host = <%= @api_host %> | ||
| port = <%= @api_port %> | ||
| logging = /etc/st2/logging.api.conf |
There was a problem hiding this comment.
why remove config option here as well?
|
LGTM for merging assuming tests pass |
Official
st2.conf: https://github.com/StackStorm/st2/blob/master/conf/st2.package.conf