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

[discuss] unify launch scripts options support #8261

Closed
colinsurprenant opened this issue Sep 14, 2017 · 9 comments · Fixed by #8285
Closed

[discuss] unify launch scripts options support #8261

colinsurprenant opened this issue Sep 14, 2017 · 9 comments · Fixed by #8285

Comments

@colinsurprenant
Copy link
Contributor

The new Windows launch scripts in #8178 is changing some behaviours from the Linux launch scripts:

  • existing JAVA_OPTS env var is now ignored, ES is also doing this
  • JAVA_TOOL_OPTIONS env var is cleared, ES is also doing this
  • now only LS_JAVA_OPTS env var is supported to append options to parsed options in the jvm.options file.
  • dropped the support of the USE_RUBY and USE_DRIP env vars. These not used anymore I believe.

My proposal is to also apply these changes to the Linux launch scripts.
WDYT?

@jordansissel
Copy link
Contributor

I still use USE_RUBY but only as a desparate workaround. I am OK if it is removed.

+1 on unifying (it should make user experience better, documentation easier, and maintenance simpler)

@original-brownbear
Copy link
Member

+1 also makes #8161 much easier (replicating the current shell behaviour there turned out to be very tricky :))

@jakelandis
Copy link
Contributor

+1 to unify, and +1 to make the 6.0.0 release, since this is non-passive.

@jordansissel
Copy link
Contributor

@jakelandis it's a bit late in the game for 6.0.0, but if we feel confident in the change (once merged), I would be OK with it landing for 6.0.0 (assuming the ship hasn't already sailed).

@suyograo
Copy link
Contributor

+1 to all your points. We should get this for 6.0 if possible.

@ph
Copy link
Contributor

ph commented Sep 14, 2017

+1 to everything , +1 for 6.0 release.

@colinsurprenant
Copy link
Contributor Author

ok, lets do this, quorum is met 😅

@colinsurprenant
Copy link
Contributor Author

PR in #8285

colinsurprenant added a commit to colinsurprenant/logstash that referenced this issue Sep 18, 2017
colinsurprenant added a commit to colinsurprenant/logstash that referenced this issue Sep 19, 2017
colinsurprenant added a commit to colinsurprenant/logstash that referenced this issue Sep 19, 2017
colinsurprenant added a commit that referenced this issue Sep 19, 2017
colinsurprenant added a commit that referenced this issue Sep 19, 2017
@colinsurprenant
Copy link
Contributor Author

#8285 new Linux launch scripts merged in master, 6.x and 6.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants