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

Use the new command line syntax in the init script #5033

Closed
wants to merge 1 commit into from

Conversation

jlecour
Copy link
Contributor

@jlecour jlecour commented Feb 6, 2014

Hi,

I've noticed that in the init script provided by the 1.0.0.RC2 Debian package, the daemon is started with pre-1.0 arguments :

DAEMON_OPTS="-d -p $PID_FILE -Des.default.config=$CONF_FILE -Des.default.path.home=$ES_HOME -Des.default.path.logs=$LOG_DIR -Des.default.path.data=$DATA_DIR -Des.default.path.work=$WORK_DIR -Des.default.path.conf=$CONF_DIR"

According to the documentation we could use a simpler/shorter invocation :

DAEMON_OPTS="-d -p $PID_FILE --default.config=$CONF_FILE --default.path.home=$ES_HOME --default.path.logs=$LOG_DIR --default.path.data=$DATA_DIR --default.path.work=$WORK_DIR --default.path.conf=$CONF_DIR"

It's definitely not a blocker, but it would be nice to be consistent.

Hi,

I've noticed that in the init script provided by the 1.0.0.RC2 Debian package, the daemon is started with pre-1.0 arguments : 

```
DAEMON_OPTS="-d -p $PID_FILE -Des.default.config=$CONF_FILE -Des.default.path.home=$ES_HOME -Des.default.path.logs=$LOG_DIR -Des.default.path.data=$DATA_DIR -Des.default.path.work=$WORK_DIR -Des.default.path.conf=$CONF_DIR"
```

According to [the documentation](http://www.elasticsearch.org/guide/en/elasticsearch/reference/1.x/_system_and_settings.html) we could use a simpler/shorter invocation : 

```
DAEMON_OPTS="-d -p $PID_FILE --default.config=$CONF_FILE --default.path.home=$ES_HOME --default.path.logs=$LOG_DIR --default.path.data=$DATA_DIR --default.path.work=$WORK_DIR --default.path.conf=$CONF_DIR"
```

It's definitely not a blocker, but it would be nice to be consistent.
@spinscale spinscale self-assigned this Mar 7, 2014
@spinscale
Copy link
Contributor

the new arguments are simply an addition and to make it easier for command-line users, but we will not remove the old style.

unsure if it makes sense to switch to the new ones or to not confuse people, when they upgrade from 0.90 here. What do you think?

@jlecour
Copy link
Contributor Author

jlecour commented Mar 7, 2014

If the core team has decided that a the new syntax is better than the old one (and I agree), why would you impose the old one to everybody?

Plus, the old syntax is pre-1.0, so people using this should be aware that anything from a pre-1.0 version is subject to change anytime. And, if someone upgrade but keep her old init script, it would still work.

In a more general point of view, I'm for adopting the best-practices, canonical syntax, … in a project whenever it's introduced. It's a signal that this is the preferred way and people cargo-culting those things at least use the correct form.

@clintongormley
Copy link

@jlecour Sorry it has taken a while to look at this.

I agree with you. Please could I ask you sign the CLA so that we can get this in?
http://www.elasticsearch.org/contributor-agreement/

thanks

@jlecour
Copy link
Contributor Author

jlecour commented Oct 20, 2014

📝 ✅

clintongormley pushed a commit that referenced this pull request Oct 28, 2014
It's definitely not a blocker, but it would be nice to be consistent.

Closes #5033
clintongormley pushed a commit that referenced this pull request Oct 28, 2014
It's definitely not a blocker, but it would be nice to be consistent.

Closes #5033
@clintongormley clintongormley changed the title Use the new command line syntax in the init script Packaging: Use the new command line syntax in the init script Nov 3, 2014
@clintongormley clintongormley changed the title Packaging: Use the new command line syntax in the init script Use the new command line syntax in the init script Jun 7, 2015
mute pushed a commit to mute/elasticsearch that referenced this pull request Jul 29, 2015
It's definitely not a blocker, but it would be nice to be consistent.

Closes elastic#5033
@mark-vieira mark-vieira added the Team:Delivery Meta label for Delivery team label Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Delivery/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts >enhancement Team:Delivery Meta label for Delivery team v1.4.0 v1.5.0 v2.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants