-
Notifications
You must be signed in to change notification settings - Fork 262
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
Mongo & Elasticsearch: extend config templates with more options #424
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @gkze for your contribution!
There's an issue with the indentation of the options you added, could you fix it?
templates/default/elastic.yaml.erb
Outdated
@@ -7,6 +7,9 @@ instances: | |||
<% unless i["pshard_stats"].nil? %> | |||
pshard_stats: <%= i["pshard_stats"] %> | |||
<% end %> | |||
<% unless i["cluster_stats"].nil? %> | |||
cluster_stats: <%= i["cluster_stats"] %> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the indentation is not correct here: cluster_stats
needs to have the same indentation as the other parameters. Could you correct it?
(same comment for the ssl
options of mongo.yaml.erb
)
Good catch @olivielpeau, thank you. Corrected. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @gkze! Looks good now 👌
The CI failures are unrelated to your changes, I'll try to find some time to fix them first and then merge your PR
For some reason I can't make the CI to re-run against a merge of this PR into the current state of master, so the build still fails. Merging this, I'll check that the resulting CI build on master is successful. Thanks @gkze! |
Thank you @olivielpeau 👍 |
We need these options in production and are currently overriding the config. We'd like to have these available to configure.