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

Drop support for CONF_FILE #13715

Closed
nik9000 opened this issue Sep 22, 2015 · 5 comments · Fixed by #13772

Comments

@nik9000
Copy link
Contributor

commented Sep 22, 2015

Right now you can configure Elasticsearch to read its configuration from either a CONF_FILE or a CONF_DIR but the CONF_FILE directive isn't consistently implemented because its default ($CONF_DIR/elasticsearch.yml) is finicky to get right.

So lets remove it in the spirit of simplification. See discussion about it in #13687 for more.

@brwe

This comment has been minimized.

Copy link
Contributor

commented Sep 22, 2015

Are we only talking about CONF_FILE (which is only for services) or also about -Des.conf=/path/to/conf.yml?

@nik9000

This comment has been minimized.

Copy link
Contributor Author

commented Sep 22, 2015

Both seem redundant. CONF_FILE is the services parlance for -Des.conf I believe.

@kimchy

This comment has been minimized.

Copy link
Member

commented Sep 22, 2015

I am +1 on removing the conf file in ES itself, and just rely on custom conf dir, but then if we can have a good logging message when someone provides it to explain that this is not used, or maybe even exit (in Bootstrap?) if it is specified.

@brwe

This comment has been minimized.

Copy link
Contributor

commented Sep 25, 2015

Turns out it is not so easy to exit in Bootstrap when people run elasticsearch service. For services convert a variable CONF_FILE to -Des.default.conf and pass this to elasticsearch. So we have to check in the init scripts too if CONF_FILE was set. Otherwise someone who configured that before will install elasticsearch, start as usual but the new installation will silently pick up the default conf.
For this reason I added a check in all init scripts in #13772 but that is very ugly as @spinscale pointed out.

We thought the following would be a trade-of:

  • push the ugly changes in #13772 with all the additional checks to 2.0 and 2.x so that there are no bad surprises for people who use the CONF_FILE option and upgrade
  • remove CONF_FILE and the additional checks completely on master
@rjernst

This comment has been minimized.

Copy link
Member

commented Sep 25, 2015

+1 to your plan @brwe

brwe added a commit to brwe/elasticsearch that referenced this issue Oct 6, 2015
…efault.conf

It is rarely used and was not consistently handled by different distributions anyway.
This commit also adds a test for specifying CONF_DIR when installing plugins and
starting elasticsearch.

relates to elastic#12712 and elastic#12954
closes elastic#5329
closes elastic#13715
@brwe brwe closed this in #13772 Oct 7, 2015
brwe added a commit that referenced this issue Oct 7, 2015
…efault.conf

It is rarely used and was not consistently handled by different distributions anyway.
This commit also adds a test for specifying CONF_DIR when installing plugins and
starting elasticsearch.

relates to #12712 and #12954
closes #5329
closes #13715

Merge pull request #13772 from brwe/remove-CONF_FILE
brwe added a commit that referenced this issue Oct 7, 2015
…efault.conf

It is rarely used and was not consistently handled by different distributions anyway.
This commit also adds a test for specifying CONF_DIR when installing plugins and
starting elasticsearch.

relates to #12712 and #12954
closes #5329
closes #13715

Merge pull request #13772 from brwe/remove-CONF_FILE
brwe added a commit that referenced this issue Oct 7, 2015
…efault.conf

It is rarely used and was not consistently handled by different distributions anyway.
This commit also adds a test for specifying CONF_DIR when installing plugins and
starting elasticsearch.

relates to #12712 and #12954
closes #5329
closes #13715

Merge pull request #13772 from brwe/remove-CONF_FILE
@clintongormley clintongormley added v2.0.0 and removed v2.1.0 labels Nov 22, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.