Windows: makes elasticsearch.bat more friendly to automated processes #9160
Conversation
|
Why everything appears as modified? Wondering what are the changes in |
|
I see... Exit... Still not understanding why git thinks everything is rewritten. Is it a CR LF issue? |
|
@dadoonet yep, CR LF. I look at it. |
|
@dadoonet thanks, should be correctly encoded now. |
|
Much better! :) |
|
|
||
| IF "!silent!" == "Y" ( | ||
| SET nopauseonerror=Y | ||
| ) ELSE ( |
There was a problem hiding this comment.
Wondering what would be the effect if you don't rewrite parameters and keep --silent as an option sent to elasticsearch?
I would probably try only to detect that -s or --silent is activated without removing it. Could this work?
There was a problem hiding this comment.
Could this work?
No. Unlike Unix script, the args name are not renamed and --silent or -s will be passed to the java exec (and failed).
There was a problem hiding this comment.
Ah. It will be seen as a java option?
There was a problem hiding this comment.
$ java -silent
Unrecognized option: -silent
Error: Could not create the Java Virtual Machine.
Error: A fatal exception has occurred. Program will exit.There was a problem hiding this comment.
But this is working:
$ java -jar injector.jar -silent
As long as - silent is at the end. Would this work with:
"%JAVA_HOME%\bin\java" %JAVA_OPTS% %ES_JAVA_OPTS% -cp "%ES_CLASSPATH%" "org.elasticsearch.bootstrap.Elasticsearch" %ES_PARAMS%
There was a problem hiding this comment.
The option --silent / -s here is intended to avoid pausing when JAVA_HOME or else is not found. What you are suggesting will move the JVM options (think -Des.node.name, -Des.pidfile etc) as program arguments, which is not the same thing.
There was a problem hiding this comment.
I did not mean that. Sorry if I was unclear. I was just trying to avoid rewriting users arguments to elasticsearch.bat. So if a user run elasticsearch --silent -Des.cluster.name=toto it will run behind the scene java -cp "%ES_CLASSPATH%" "org.elasticsearch.bootstrap.Elasticsearch" --silent -Des.cluster.name=toto but --silent will be totally ignored by elasticsearch...
Not a big deal to me. Just trying to have less code to maintain :)
There was a problem hiding this comment.
but --silent will be totally ignored by elasticsearch...
And also all -D properties which will be passed as program argument and not as Java command line options...
There was a problem hiding this comment.
I see. So forget all my comments! :)
|
Left a comment. The change looks good to me though I did not test it on a windows machine. If you need me to test it, I can probably do it later this week. |
|
Just tested this on my Windows 8.1 machine and it looks good. 👍 |
|
I wonder though if this should just be the default behavior rather than needing a flag? To me, it's more important that the script be programmable out of the box rather than being "manual friendly". |
On Windows platforms when JAVA_HOME is not defined, a message is printed on standard output and the bat script is paused until the user press a key. This behavior is not compliant to automated processes where elasticsearch.bat can be executed by another script. This commit adds a new parameter --silent / -s that allow to skip the pause. Also, the error message is directed to standard and error outputs. Closes elastic#8913
clintongormley
added
:Delivery/Packaging
When JAVA_HOME is not defined on a Windows platform, a message is printed on standard output and the BAT script is paused until the user press a key. This behavior is not compliant to automated processes where elasticsearch.bat can be executed by another script. This commit adds a new parameter --silent / -s that allow to skip this pause. Also, the error message is directed to standard and error outputs.
Closes #8913