Skip to content
This repository was archived by the owner on Jun 20, 2024. It is now read-only.

Add a global configuration option to control when to fall back to system #350

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all 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
53 changes: 32 additions & 21 deletions core/src/main/scala/dagr/core/config/Configuration.scala
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,16 @@ object Configuration extends ConfigurationLike {

// Keys for configuration values used in dagr core
object Keys {
val CommandLineName = "dagr.command-line-name"
val ColorStatus = "dagr.color-status"
val SystemPath = "dagr.path"
val PackageList = "dagr.package-list"
val ScriptDirectory = "dagr.script-directory"
val LogDirectory = "dagr.log-directory"
val SystemCores = "dagr.system-cores"
val SystemMemory = "dagr.system-memory"
val PrintArgs = "dagr.print-args"
val CommandLineName = "dagr.command-line-name"
val ColorStatus = "dagr.color-status"
val SystemPath = "dagr.path"
val PackageList = "dagr.package-list"
val ScriptDirectory = "dagr.script-directory"
val LogDirectory = "dagr.log-directory"
val SystemCores = "dagr.system-cores"
val SystemMemory = "dagr.system-memory"
val PrintArgs = "dagr.print-args"
val FallBackToSystemPath = "dagr.fall-back-to-system-path"
Copy link
Member

Choose a reason for hiding this comment

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

fall-back-to-system-path-for-missing-executables

?

}

// The global configuration instance
Expand Down Expand Up @@ -165,25 +166,40 @@ private[config] trait ConfigurationLike extends LazyLogging {
}
}

/** True to fall back to the searching the system path when a configuration key exists but the executable does not.
* False to throw an exception if the configuration key exists but the executable does not. */
private def fallBackToSystemPath: Boolean = configure(Configuration.Keys.FallBackToSystemPath, false)
Copy link
Member Author

Choose a reason for hiding this comment

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

I have to think about if this could be a lazy val (i.e. is config immutable)


/**
* Attempts to determine the path to an executable, first by looking it up in config,
* and if that fails, by attempting to locate it on the system path. If both fail then
* an exception is raised.
*
* If the configuration key exists but the value (i.e. executable path) does not, the fallback behavior is controlled
* by the `Configuration.Keys.FallBackToSystemPath` configuration value (default is false). If false, an exception
* will be raised if the configuration key exists but the executable (value) does not exist, otherwise the system
* path is searched.
*
* @param path the configuration path to look up
* @param executable the default name of the executable
* @return An absolute path to the executable to use
*/
def configureExecutable(path: String, executable: String) : Path = {
Configuration.RequestedKeys += path

optionallyConfigure[Path](path) match {
case Some(exec) => exec
case None => findInPath(executable) match {
case Some(exec) => exec
case None => throw new Generic(s"Could not configurable executable. Config path '$path' is not defined and executable '$executable' is not in PATH.")
}
}
optionallyConfigure[Path](path)
// always keep the value if we should not fall back, otherwise if we should fallback, keep the value if the file
// exists
.filter(!fallBackToSystemPath || Files.exists(_))
.orElse(findInPath(executable))
.map(_.toAbsolutePath)
.getOrElse(
throw new Generic(
s"Could not configure executable. Config path `$path` is not defined "
+ (if (fallBackToSystemPath) "" else s"or the config value references a file which does not exist, ")
+ s"and executable '$executable' is not on the system path."
)
)
}

/**
Expand Down Expand Up @@ -224,11 +240,6 @@ private[config] trait ConfigurationLike extends LazyLogging {
*/
protected def systemPath : Seq[Path] = config.getString(Configuration.Keys.SystemPath).split(File.pathSeparatorChar).view.map(pathTo(_))

/** Removes various characters from the simple class name, for scala class names. */
private def sanitizeSimpleClassName(className: String): String = {
className.replaceFirst("[$].*$", "")
}

/** Searches the system path for the executable and return the full path. */
private def findInPath(executable: String) : Option[Path] = {
systemPath.map(p => p.resolve(executable)).find(ex => Files.exists(ex))
Expand Down
21 changes: 20 additions & 1 deletion core/src/test/scala/dagr/core/config/ConfigurationTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ class ConfigurationTest extends UnitSpec with CaptureSystemStreams {
pw.println(Configuration.Keys.ColorStatus + " = false")
pw.close()

val conf = new Configuration { override val config : Config = ConfigFactory.parseFile(configPath.toFile) }
val conf: Configuration = new Configuration { override val config : Config = ConfigFactory.parseFile(configPath.toFile).resolve() }
conf.configure[String](Configuration.Keys.CommandLineName) shouldBe "command-line-name"
conf.configure[Boolean](Configuration.Keys.ColorStatus) shouldBe false
conf.optionallyConfigure[String](Configuration.Keys.SystemPath) shouldBe 'empty
Expand All @@ -149,6 +149,25 @@ class ConfigurationTest extends UnitSpec with CaptureSystemStreams {
Configuration.requestedKeys should contain("java.exe")
}

it should s"fallback to the system path when ${Configuration.Keys.FallBackToSystemPath} is true" in {
val configPath = Files.createTempFile("config", ".txt")
configPath.toFile.deleteOnExit()

val pw = new PrintWriter(Io.toWriter(configPath))
pw.println(Configuration.Keys.SystemPath + " = ${PATH}")
pw.println(Configuration.Keys.FallBackToSystemPath + " = true")
pw.close()

val conf: Configuration = new Configuration { override val config : Config = ConfigFactory.parseFile(configPath.toFile).resolve() }

conf.configure[Boolean](Configuration.Keys.FallBackToSystemPath) shouldBe true

var java = conf.configureExecutable("java.exe", "java")
java = conf.configureExecutable("some-executable", "java")
java.getFileName.toString shouldBe "java"
Files.isExecutable(java) shouldBe true
}

it should "thrown an exception when an executable cannot be found" in {
an[ConfigException] should be thrownBy conf.configureExecutable("some-executable-not-found", "n/a")
an[ConfigException] should be thrownBy conf.configureExecutableFromBinDirectory("some-bin-dir-not-found", "n/a")
Expand Down