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

drop default JAVA_HOME from SparkLauncher #31

Conversation

faucct
Copy link
Collaborator

@faucct faucct commented Sep 30, 2024

This forces the java to be at specific path, though it is probably enough for it to be resolvable via $PATH.

@faucct faucct requested a review from alextokarew September 30, 2024 08:04
@faucct faucct force-pushed the feature/drop-default-JAVA_HOME-from-SparkLauncher branch from 6564288 to 384a5bf Compare September 30, 2024 11:12

private val livyHome: String = absolutePath(env("LIVY_HOME", "./livy"))

private val javaHome: String = absolutePath(env("JAVA_HOME", null))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why JAVA_HOME is set to null by default?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So working java executable would not be broken by a non-existent /opt/jdk11 path.

val startProcess = Process(
livyRunner,
new File("."),
val extraEnv = ArrayBuffer(
"SPARK_HOME" -> sparkHome,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Actually, you're not dropping SPARK_HOME, as you wrote in changeset description.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

My bad, has renamed the PR.

@faucct faucct changed the title drop default SPARK_HOME from SparkLauncher drop default JAVA_HOME from SparkLauncher Oct 1, 2024
Copy link

robot-magpie bot commented Oct 8, 2024

@alextokarew has imported your pull request. If you are a Yandex employee, you can view this diff.

Copy link

robot-magpie bot commented Oct 8, 2024

✅ This pull request is being closed because it has been successfully merged into our internal monorepository.
Your changes will be pushed to this repository soon. Thank you for your contribution!

@robot-magpie robot-magpie bot closed this Oct 8, 2024
robot-piglet pushed a commit that referenced this pull request Oct 8, 2024
This forces the java to be at specific path, though it is probably enough for it to be resolvable via `$PATH`.

---

Pull Request resolved: #31
commit_hash:ac3400c8fb8931aab3afbd099c82b21c72ac7999
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants