Skip to content

Commit

Permalink
Fix querying on empty runners & engines (#691)
Browse files Browse the repository at this point in the history
* Fix querying on empty runners & engines

When applying a filter on empty runners or engines, the fallback default `native engine` and `executable runner` respectively are set before applying the filter

* Add tests for findEngines and findRunners
  • Loading branch information
Grifs authored Apr 26, 2024
1 parent 3075bc6 commit 0f242fc
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 36 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ TODO add summary
* `MainNSBuildNativeSuite`: Capture the error message when reading the configs so we can capture the expected warning message (PR #688).
While almost all tests were already cleanly capturing their expected warning/error messages, this one was still remaining, resulting in warnings being shown in the output.

* `runners & engines`: When applying a filter on empty runners or engines, the fallback default `native engine` and `executable runner` respectively are set before applying the filter (PR #691).

# Viash 0.9.0-RC2 (2024-02-23): Restructure the config and change some default values

The `.functionality` layer has been removed from the config and all fields have been moved to the top layer.
Expand Down
70 changes: 34 additions & 36 deletions src/main/scala/io/viash/config/Config.scala
Original file line number Diff line number Diff line change
Expand Up @@ -414,59 +414,57 @@ case class Config(
* Find the runners
*
* Order of execution:
* - if an runner id is passed, look up the runner in the runners list
* - else if runners is a non-empty list, use the first runner
* - else use the executable runner
* - if the runners list is empty, use the executable runner for the rest of the logic
* - if an runner id is passed, return the matching runners in the runners list
* - else throw an error
*
* @param query An runner ID referring to one of the config's runners
* @return An runner
* @return A list of runners
*/
def findRunners(query: Option[String]): List[Runner] = {
// TODO: match on query, there's no need to do a .* if query is None
val regex = query.getOrElse(".*").r

val foundMatches = runners.filter{ e =>
regex.findFirstIn(e.id).isDefined
val list = runners match {
case Nil => List(ExecutableRunner())
case li => li
}

foundMatches match {
case li if li.nonEmpty =>
li
case Nil if query.isDefined =>
throw new RuntimeException(s"no runner id matching regex '$regex' could not be found in the config.")
case _ =>
// TODO: switch to the getRunners.head ?
List(ExecutableRunner())

query match {
case None =>
list
case Some(regex) =>
val foundMatches = list.filter{ e => regex.r.findFirstIn(e.id).isDefined }
foundMatches match {
case li if li.nonEmpty => li
case _ => throw new RuntimeException(s"no runner id matching regex '$regex' could not be found in the config.")
}
}
}

/**
* Find the engines
*
* Order of execution:
* - if an engine id is passed, look up the engine in the engines list
* - else if engines is a non-empty list, use the first engine
* - else use the executable engine
* - if the engines list is empty, use the native engine for the rest of the logic
* - if an engine id is passed, return the matching engines in the engines list
* - else throw an error
*
* @param query An engine ID referring to one of the config's engines
* @return An engine
* @return A list of engines
*/
def findEngines(query: Option[String]): List[Engine] = {
// TODO: match on query, there's no need to do a .* if query is None
val regex = query.getOrElse(".*").r

val foundMatches = engines.filter{ e =>
regex.findFirstIn(e.id).isDefined
val list = engines match {
case Nil => List(NativeEngine())
case li => li
}

foundMatches match {
case li if li.nonEmpty =>
li
case Nil if query.isDefined =>
throw new RuntimeException(s"no engine id matching regex '$regex' could not be found in the config.")
case _ =>
// TODO: switch to the getEngines.head ?
List(NativeEngine())

query match {
case None =>
list
case Some(regex) =>
val foundMatches = list.filter{ e => regex.r.findFirstIn(e.id).isDefined }
foundMatches match {
case li if li.nonEmpty => li
case _ => throw new RuntimeException(s"no engine id matching regex '$regex' could not be found in the config.")
}
}
}

Expand Down
47 changes: 47 additions & 0 deletions src/test/scala/io/viash/config/ConfigTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import io.circe.syntax._
import io.viash.helpers.circe._
import io.viash.helpers.data_structures._
import io.viash.helpers.Logger
import io.viash.engines.NativeEngine
import io.viash.runners.ExecutableRunner

class ConfigTest extends AnyFunSuite with BeforeAndAfterAll {
Logger.UseColorOverride.value = Some(false)
Expand Down Expand Up @@ -47,5 +49,50 @@ class ConfigTest extends AnyFunSuite with BeforeAndAfterAll {
assert(confParsed == conf)
}

test("GetEngines and GetRunners with some engines and runners") {
val conf = Config(
name = "foo",
engines = List(
NativeEngine("engine0"),
NativeEngine("engine1"),
NativeEngine("engine2")
),
runners = List(
ExecutableRunner("runner0"),
ExecutableRunner("runner1"),
ExecutableRunner("runner2")
)
)

assert(conf.findEngines(None) == conf.engines)
assert(conf.findRunners(None) == conf.runners)

assert(conf.findEngines(Some(".*")) == conf.engines)
assert(conf.findRunners(Some(".*")) == conf.runners)

assert(conf.findEngines(Some("engine0")) == List(NativeEngine("engine0")))
assert(conf.findEngines(Some("engine1")) == List(NativeEngine("engine1")))
assert(conf.findRunners(Some("runner0")) == List(ExecutableRunner("runner0")))
assert(conf.findRunners(Some("runner1")) == List(ExecutableRunner("runner1")))

assert(conf.findEngines(Some(".*0")) == List(NativeEngine("engine0")))
assert(conf.findEngines(Some(".*1")) == List(NativeEngine("engine1")))
assert(conf.findRunners(Some(".*0")) == List(ExecutableRunner("runner0")))
assert(conf.findRunners(Some(".*1")) == List(ExecutableRunner("runner1")))
}

test("GetEngines and GetRunners without engines or runners") {
val conf = Config(name = "foo")

assert(conf.findEngines(None) == List(NativeEngine("native")))
assert(conf.findRunners(None) == List(ExecutableRunner("executable")))

assert(conf.findEngines(Some(".*")) == List(NativeEngine("native")))
assert(conf.findRunners(Some(".*")) == List(ExecutableRunner("executable")))

assert(conf.findEngines(Some("native")) == List(NativeEngine("native")))
assert(conf.findRunners(Some("executable")) == List(ExecutableRunner("executable")))
}

// TODO: expand functionality tests
}

0 comments on commit 0f242fc

Please sign in to comment.