davsclaus commented on code in PR #27472:
URL: https://github.com/apache/camel/pull/27472#discussion_r4203984748
##########
dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/infra/InfraRun.java:
##########
@@ -301,12 +319,34 @@ protected Integer doRun(String testService, String
testServiceImplementation, Te
// ignore
}
- shutdownInfra(closed, logFile, jsonFile, actualService);
+ shutdownInfra(closed, logFile, jsonFile, actualService,
serviceProperties);
return 0;
}
- private static void shutdownInfra(AtomicBoolean closed, Path logFile, Path
jsonFile, Object actualService) {
+ /**
+ * Turns the --property options into system properties, which is how a
service takes an option the CLI has no flag
+ * of its own for, such as the model of ollama.
+ *
+ * @return the names that were set, to be cleared when the service stops
+ */
+ List<String> setServiceProperties() {
+ List<String> names = new ArrayList<>(properties.size());
+ for (String property : properties) {
+ int separator = property.indexOf('=');
+ if (separator < 1) {
Review Comment:
`separator < 1` only checks that there is a name; `ollama.model=` sets an
empty model. Also check `separator == property.length() - 1`.
##########
dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/infra/InfraRun.java:
##########
@@ -179,6 +187,15 @@ protected Integer doRun(String testService, String
testServiceImplementation, Te
if (noUi) {
System.setProperty("camel.infra.ui", "false");
}
+ // the service resolves its own properties from the system properties
first, so they must be set
+ // before it is instantiated
Review Comment:
`run()` sends `--background` to `runBackground()` before `doRun` is reached,
so this check never runs in background mode. The background branch returns
before `setServiceProperties()` runs in `doRun`. Could the properties be parsed
and validated up front (e.g. in `doCall()`), returning a map that `doRun` only
applies?
##########
docs/user-manual/modules/ROOT/pages/camel-jbang-dev-services.adoc:
##########
@@ -64,6 +64,25 @@ Starting service kafka with implementation redpanda
}
----
+== Configuring a service
+
+A service reads its own settings from system properties, so `--property`
passes them through. The
+option is repeatable and takes `key=value`:
+
+[source,bash]
+----
+$ camel infra run ollama --property ollama.model=qwen2.5:0.5b
+
+Starting service ollama
+----
+
+Ollama pulls `granite4:3b` by default, 2.1 GB; `qwen2.5:0.5b` is around 400
MB, which is enough for
+most development and a lot faster to start. The same option sets the embedding
model
+(`ollama.embedding.model`), the container limits (`ollama.container.cpu.count`,
+`ollama.container.memory.limit`), GPU support (`ollama.container.enable.gpu`,
`enabled` or
Review Comment:
`ollama.container.cpu.count` and `ollama.container.memory.limit` are only
constants in `OllamaProperties`; nothing in test-infra reads them. Remove them
here (or wire them up in `OllamaLocalContainerInfraService`). Also, the default
model and its size on line 79 will go stale when `container.properties` changes.
##########
dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/infra/InfraRun.java:
##########
@@ -179,6 +187,15 @@ protected Integer doRun(String testService, String
testServiceImplementation, Te
if (noUi) {
System.setProperty("camel.infra.ui", "false");
}
+ // the service resolves its own properties from the system properties
first, so they must be set
+ // before it is instantiated
+ List<String> serviceProperties;
+ try {
+ serviceProperties = setServiceProperties();
+ } catch (IllegalArgumentException e) {
+ printer().printErr(e.getMessage());
+ return 1;
Review Comment:
On this early return (and if a later step such as `initialize()` throws) the
properties set so far, and `camel.infra.*`, are not cleared, since the cleanup
is only registered after `initialize()`. Setting them all only after validating
everything, and clearing them in a `finally`, would avoid the leak for same-JVM
callers (tests, `InfraRestart`).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
