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

Open Java modules for Snowflake setup #9664

Merged
merged 7 commits into from
Apr 16, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,10 @@ type Snowflake_Details
Provides the properties for the connection.
jdbc_properties : Vector (Pair Text Text)
jdbc_properties self =
## Avoid the Arrow dependency (https://community.snowflake.com/s/article/SAP-BW-Java-lang-NoClassDefFoundError-for-Apache-arrow)
no_arrow = [Pair.new 'jdbc_query_result_format' 'json']
## If Arrow dependency is to be avoided (https://community.snowflake.com/s/article/SAP-BW-Java-lang-NoClassDefFoundError-for-Apache-arrow)
## [Pair.new 'jdbc_query_result_format' 'json']
## should be prepended to properties. That would make it fallback to plain `json`.

account = [Pair.new 'account' self.account]
credentials = [Pair.new 'user' self.credentials.username, Pair.new 'password' self.credentials.password]
database = [Pair.new 'db' self.database]
Expand All @@ -55,4 +57,4 @@ type Snowflake_Details

## Control the format of TIMESTAMP and TIME fields
formats = [Pair.new "TIME_OUTPUT_FORMAT" "HH24:MI:SS.FF9", Pair.new "TIMESTAMP_OUTPUT_FORMAT" "YYYY-MM-DD HH24:MI:SS.FF9 TZHTZM", Pair.new "TIMESTAMP_NTZ_OUTPUT_FORMAT" "YYYY-MM-DD HH24:MI:SS.FF9", Pair.new "TIMESTAMP_LTZ_OUTPUT_FORMAT" "YYYY-MM-DD HH24:MI:SS.FF9"]
no_arrow + account + credentials + database + schema + warehouse + formats
account + credentials + database + schema + warehouse + formats
23 changes: 15 additions & 8 deletions engine/launcher/src/main/scala/org/enso/launcher/Launcher.scala
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ case class Launcher(cliOptions: GlobalCLIOptions) {
additionalArguments = additionalArguments
)
.get,
JVMSettings(useSystemJVM, jvmOpts)
JVMSettings(useSystemJVM, jvmOpts, extraOptions = Seq.empty)
) { command =>
command.run().get
}
Expand Down Expand Up @@ -223,7 +223,7 @@ case class Launcher(cliOptions: GlobalCLIOptions) {
additionalArguments
)
.get,
JVMSettings(useSystemJVM, jvmOpts)
JVMSettings(useSystemJVM, jvmOpts, extraOptions = Seq())
) { command =>
command.run().get
}
Expand Down Expand Up @@ -267,7 +267,7 @@ case class Launcher(cliOptions: GlobalCLIOptions) {
additionalArguments
)
.get,
JVMSettings(useSystemJVM, jvmOpts)
JVMSettings(useSystemJVM, jvmOpts, extraOptions = Seq())
) { command =>
command.run().get
}
Expand Down Expand Up @@ -310,7 +310,7 @@ case class Launcher(cliOptions: GlobalCLIOptions) {
additionalArguments
)
.get,
JVMSettings(useSystemJVM, jvmOpts)
JVMSettings(useSystemJVM, jvmOpts, Seq())
) { command =>
command.run().get
}
Expand Down Expand Up @@ -346,7 +346,7 @@ case class Launcher(cliOptions: GlobalCLIOptions) {
additionalArguments = additionalArguments
)
.get,
JVMSettings(useSystemJVM, jvmOpts)
JVMSettings(useSystemJVM, jvmOpts, extraOptions = Seq())
) { command =>
command.run().get
}
Expand Down Expand Up @@ -413,8 +413,11 @@ case class Launcher(cliOptions: GlobalCLIOptions) {
)
.get

runner.withCommand(settings, JVMSettings(useSystemJVM, jvmOpts)) {
command => command.run().get
runner.withCommand(
settings,
JVMSettings(useSystemJVM, jvmOpts, extraOptions = Seq())
) { command =>
command.run().get
}
}

Expand Down Expand Up @@ -530,7 +533,11 @@ case class Launcher(cliOptions: GlobalCLIOptions) {
val runtimeVersionString = if (isEngineInstalled) {
val output = runner.withCommand(
runtimeVersionRunSettings,
JVMSettings(useSystemJVM = false, jvmOptions = Seq.empty)
JVMSettings(
useSystemJVM = false,
jvmOptions = Seq(),
extraOptions = Seq()
)
) { runtimeVersionCommand =>
runtimeVersionCommand.captureOutput().get
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,15 +92,23 @@ class LauncherRunnerSpec extends RuntimeVersionManagerTest with FlakySpec {

runner.withCommand(
runSettings,
JVMSettings(useSystemJVM = true, jvmOptions = jvmOptions)
JVMSettings(
useSystemJVM = true,
jvmOptions = jvmOptions,
extraOptions = Seq()
)
) { systemCommand =>
systemCommand.command.head shouldEqual "java"
checkCommandLine(systemCommand)
}

runner.withCommand(
runSettings,
JVMSettings(useSystemJVM = false, jvmOptions = jvmOptions)
JVMSettings(
useSystemJVM = false,
jvmOptions = jvmOptions,
extraOptions = Seq()
)
) { managedCommand =>
managedCommand.command.head should include("java")
val javaHome =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,8 @@ class TestDistributionConfiguration(
val javaCommand = JavaCommand(currentProcess, None)
new JVMSettings(
javaCommandOverride = Some(javaCommand),
jvmOptions = Seq()
jvmOptions = Seq(),
extraOptions = Seq()
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ class ProjectCreateHandleMissingRuntimeSpec
) {
override def defaultJVMSettings: JVMSettings = JVMSettings(
javaCommandOverride = None,
jvmOptions = Seq()
jvmOptions = Seq(),
extraOptions = Seq()
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,13 @@ package org.enso.runtimeversionmanager.runner
* instead of the default JVM provided with the
* release; it can be an absolute path to a java
* executable
* @param jvmOptions options that should be added to the launched JVM
* @param jvmOptions options that should be added to the launched JVM, will be prefixed with `-D`
* @param extraOptions extra options that should be added to the launched JVM
*/
case class JVMSettings(
javaCommandOverride: Option[JavaCommand],
jvmOptions: Seq[(String, String)]
jvmOptions: Seq[(String, String)],
extraOptions: Seq[(String, String)]
)

object JVMSettings {
Expand All @@ -19,20 +21,31 @@ object JVMSettings {
*
* @param useSystemJVM if set, the system configured JVM is used instead of
* the one managed by the launcher
* @param jvmOptions options that should be added to the launched JVM
* @param jvmOptions options that should be added to the launched JVM, will be prefixed with `-D`
* @param extraOptions extra options that should be added to the launched JVM
*/
def apply(
useSystemJVM: Boolean,
jvmOptions: Seq[(String, String)]
jvmOptions: Seq[(String, String)],
extraOptions: Seq[(String, String)]
): JVMSettings =
new JVMSettings(
if (useSystemJVM) Some(JavaCommand.systemJavaCommand) else None,
jvmOptions
jvmOptions,
extraOptions
)

// See propositions in #9475 for alternatives
private val nioOpen: (String, String) =
("add-opens", "java.base/java.nio=ALL-UNNAMED")

/** Creates a default instance of [[JVMSettings]] that just use the default
* JVM with no options overrides.
*/
def default: JVMSettings =
JVMSettings(useSystemJVM = false, jvmOptions = Seq())
JVMSettings(
useSystemJVM = false,
jvmOptions = Seq(),
extraOptions = Seq(nioOpen)
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -168,10 +168,13 @@ class Runner(
)
}

def translateJVMOption(option: (String, String)): String = {
def translateJVMOption(
option: (String, String),
standardOption: Boolean
): String = {
val name = option._1
val value = option._2
s"-D$name=$value"
if (standardOption) s"-D$name=$value" else s"--$name=$value"
}

val context = JVMOptionsContext(enginePackagePath = engine.path)
Expand All @@ -180,7 +183,12 @@ class Runner(
engine.defaultJVMOptions.filter(_.isRelevant).map(_.substitute(context))
val environmentOptions =
jvmOptsFromEnvironment.map(_.split(' ').toIndexedSeq).getOrElse(Seq())
val commandLineOptions = jvmSettings.jvmOptions.map(translateJVMOption)
val commandLineOptions = jvmSettings.jvmOptions.map(
translateJVMOption(_, standardOption = true)
) ++
jvmSettings.extraOptions.map(
translateJVMOption(_, standardOption = false)
)
val shouldInvokeViaModulePath = engine.graalRuntimeVersion.isUnchained

var jvmArguments =
Expand Down
Loading