-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Clarify missing java error message #46160
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
Conversation
Since the bundled jdk was added to Elasticsearch, there are now 2 ways java can be missing. Either JAVA_HOME is set but does not exist, or the bundled jdk does not exist. This commit improves the error messages in those two cases, and also ensures our tests cover both cases.
|
Pinging @elastic/es-core-infra |
|
@elasticmachine, run elasticsearch-ci/docs |
pugnascotia
left a comment
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 changes look good.
I wondered if we should sync the control flow of elasticsearch-env and elasticsearch-env.bat, since the former checks whether JAVA is executable in both the bundled and non-bundled branches, whereas the latter has a check after both branches.
Since the bundled jdk was added to Elasticsearch, there are now 2 ways java can be missing. Either JAVA_HOME is set but does not exist, or the bundled jdk does not exist. This commit improves the error messages in those two cases, and also ensures our tests cover both cases.
|
UPDATE: figured it out. that user didn't own the entire file path. I'll leave the rest in case others run into the issue. I still see this error in 7.11.2 ./elasticsearch sestatus My user owns those files and directories, so not sure why it couldn't find it |
Since the bundled jdk was added to Elasticsearch, there are now 2 ways
java can be missing. Either JAVA_HOME is set but does not exist, or the
bundled jdk does not exist. This commit improves the error messages in
those two cases, and also ensures our tests cover both cases.
closes #44139