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

Resolve JAVA_HOME for source builds lazily #1027

Merged
merged 2 commits into from
Jul 20, 2020

Conversation

danielmitterdorfer
Copy link
Member

With this commit we resolve JAVA_HOME lazily when attempting to build
Elasticsearch. When a source build is externally provided to Rally's
source artifact cache, there is no need to actually build it. However,
if the target system does not have any JDK installed and we attempt to
install Elasticsearch, the build subsystem is eagerly initialized
(despite the artifact being already cached) and is complaining that it
cannot find a JDK to build the artifact. By lazily resolving JAVA_HOME
we avoid that error.

With this commit we resolve JAVA_HOME lazily when attempting to build
Elasticsearch. When a source build is externally provided to Rally's
source artifact cache, there is no need to actually build it. However,
if the target system does not have any JDK installed and we attempt to
install Elasticsearch, the build subsystem is eagerly initialized
(despite the artifact being already cached) and is complaining that it
cannot find a JDK to build the artifact. By lazily resolving JAVA_HOME
we avoid that error.
@danielmitterdorfer danielmitterdorfer added bug Something's wrong :Benchmark Candidate Management Anything affecting how Rally sets up Elasticsearch labels Jul 16, 2020
@danielmitterdorfer danielmitterdorfer added this to the 2.0.1 milestone Jul 16, 2020
@danielmitterdorfer danielmitterdorfer self-assigned this Jul 16, 2020
Copy link
Contributor

@hub-cap hub-cap left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense to me, LGTM.

try:
build_jdk = int(raw_build_jdk)
except ValueError:
raise exceptions.SystemSetupError(f"Car config key \"build.jdk\" is invalid: \"{raw_build_jdk}\" (must be int)")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW im perfectly fine if we use single quotes inside f strings for things like this... f"blah 'this thing' blah", I hate seeing \ when we can get around it :)

If you dont agree, thats fine by me!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Usually we use square brackets anyway instead of double quotes. I've pushed a change in 690d568.

@danielmitterdorfer danielmitterdorfer merged commit ec2f8a2 into elastic:master Jul 20, 2020
@danielmitterdorfer danielmitterdorfer deleted the lazy-java-home branch July 20, 2020 06:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Benchmark Candidate Management Anything affecting how Rally sets up Elasticsearch bug Something's wrong
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants