Launch Manager loads the new configuration file by paulquiring · Pull Request #248 · eclipse-score/lifecycle

paulquiring · 2026-06-15T14:18:09Z

Map user-facing json config to the new flatbuffer config
Load new launch manager config
Introduce adapters to make the rest of the code work with the new configuration content
Changes are avaible using --//config:use_new_configuration=True

Closes: #209

github-actions · 2026-06-15T14:19:10Z

License Check Results

🚀 The license check job ran with the Bazel command:

bazel run --lockfile_mode=error //:license-check

Status: ⚠️ Needs Review

Click to expand output

[License Check Output]
Extracting Bazel installation...
Starting local Bazel server (8.6.0) and connecting to it...
INFO: Invocation ID: 2910de83-6173-41d1-8ead-28f84225e62c
Computing main repo mapping: 
Computing main repo mapping: 
Loading: 
Loading: 0 packages loaded
Loading: 0 packages loaded
Loading: 0 packages loaded
    currently loading: 
Analyzing: target //:license-check (1 packages loaded)
Analyzing: target //:license-check (1 packages loaded, 0 targets configured)
Analyzing: target //:license-check (1 packages loaded, 0 targets configured)

Analyzing: target //:license-check (36 packages loaded, 10 targets configured)

Analyzing: target //:license-check (129 packages loaded, 118 targets configured)

Analyzing: target //:license-check (152 packages loaded, 5481 targets configured)

Analyzing: target //:license-check (160 packages loaded, 6596 targets configured)

Analyzing: target //:license-check (163 packages loaded, 8263 targets configured)

Analyzing: target //:license-check (164 packages loaded, 8271 targets configured)

Analyzing: target //:license-check (165 packages loaded, 8273 targets configured)

Analyzing: target //:license-check (167 packages loaded, 10159 targets configured)

Analyzing: target //:license-check (168 packages loaded, 10283 targets configured)

INFO: Analyzed target //:license-check (169 packages loaded, 10409 targets configured).
[14 / 16] [Prepa] JavaToolchainCompileBootClasspath external/rules_java+/toolchains/platformclasspath.jar
INFO: Found 1 target...
Target //:license.check.license_check up-to-date:
  bazel-bin/license.check.license_check
  bazel-bin/license.check.license_check.jar
INFO: Elapsed time: 21.894s, Critical Path: 2.13s
INFO: 16 processes: 12 internal, 3 processwrapper-sandbox, 1 worker.
INFO: Build completed successfully, 16 total actions
INFO: Running command line: bazel-bin/license.check.license_check ./formatted.txt <args omitted>
usage: org.eclipse.dash.licenses.cli.Main [-batch <int>] [-cd <url>]
       [-confidence <int>] [-ef <url>] [-excludeSources <sources>] [-help] [-lic
       <url>] [-project <shortname>] [-repo <url>] [-review] [-summary <file>]
       [-timeout <seconds>] [-token <token>]

S-MOHAMD · 2026-06-17T16:47:59Z

Please lnk corresponding issue/bug.
see: https://docs.github.com/en/issues/tracking-your-work-with-issues/using-issues/linking-a-pull-request-to-an-issue

github-actions · 2026-06-18T08:33:36Z

The created documentation from the pull request is available at: docu-html

NicolasFussberger · 2026-06-24T13:31:32Z

+    bool is_sup = (props.application_profile.application_type == ApplicationType::ReportingAndSupervised ||
+                    props.application_profile.application_type == ApplicationType::StateManager);
+    if (is_sup && env_index < static_cast<size_t>(score::lcm::internal::kMaxEnv)) {
+        std::string iface_path = "LCM_ALIVE_INTERFACE_PATH=" + score::lcm::internal::aliveInterfacePath(comp.name);


we could also add a constant for the env name

NicolasFussberger · 2026-06-24T13:38:17Z

+                    ? score::lcm::ProcessState::kRunning
+                    : score::lcm::ProcessState::kTerminated;
+            }
+        }


here is a possible error case. In case the configured dependency does not actually exist

NicolasFussberger · 2026-06-24T13:39:30Z

+        pgm.number_of_restart_attempts = deploy.ready_recovery_action->number_of_attempts;
+    }
+
+    for (const auto& dep_name : props.depends_on) {


depends_on might also contain a run_target and not only a component.
Special Case: This may be also the fallback_run_target

NicolasFussberger · 2026-06-24T13:41:23Z

+            }
+        }
+        auto rt_it = run_target_by_name.find(dep_name);
+        if (rt_it != run_target_by_name.end()) {


I think depends_on might also refer to fallback_run_target which is currently not in the run_target_by_name map

NicolasFussberger · 2026-06-24T13:46:56Z

+        run_target_by_name[rt.name] = &rt;
+    }
+
+    std::map<std::string, const ComponentConfig*> component_by_name;


component_by_name exists twice in this class

NicolasFussberger · 2026-06-24T13:52:52Z

+                          std::set<std::string>& visited) {
+        if (!visited.insert(dep_name).second) return;
+        auto comp_it = component_to_process_index.find(dep_name);
+        if (comp_it != component_to_process_index.end()) {


There might be some possible error cases when component/runTarget names are not found because they have not been configured

Maybe we keep the current validation in the python script and just put asserts in the code for these error cases

NicolasFussberger · 2026-06-24T13:53:14Z

+        component_by_name[comp.name] = &comp;
+    }
+
+    std::function<void(const std::string&, std::vector<uint32_t>&, std::set<std::string>&)> resolve_depends;


Maybe it could make sense to have a dedicated Visitor class for this

NicolasFussberger · 2026-06-24T14:04:58Z

+}
+
+void ConfigurationAdapter::resolveDependencyIndexes(std::vector<OsProcess>& processes) {
+    for (auto& proc : processes) {


Is this needed?

NicolasFussberger · 2026-06-24T14:08:36Z

+        component_by_name[comp.name] = &comp;
+    }
+
+    std::map<std::string, uint32_t> component_to_process_index;


should we build these maps once during initialize and store them as private member?

NicolasFussberger · 2026-06-24T14:15:40Z

+    if (pg) {
+        if (index < pg->processes_.size()) {
+            return pg;
+        }


does not make sense to validate the index here

NicolasFussberger · 2026-06-24T14:16:32Z

+
+    static const uint32_t kDefaultProcessExecutionError;
+    static uint32_t kDefaultProcessorAffinityMask();
+    static const int32_t kDefaultSchedulingPolicy;


These defaults seem obsolete now

- Load new launch manager config - Introduce adapters to make the rest of the code work with the new configuration content - Changes are avaible using --//config:use_new_configuration=True

NicolasFussberger · 2026-07-06T09:16:32Z

+bool_flag(
+    name = "use_new_configuration",
+    build_setting_default = False,
+    visibility = ["//visibility:public"],


Should we make this visibility private?
I assume this is an internal flag for development

NicolasFussberger · 2026-07-06T09:34:56Z

-    /// @return Pointer to the ConfigurationManager object.
-    ConfigurationManager* getConfigurationManager();
+    /// @brief Gets the configuration.
+    /// @return Pointer to the configuration object.


I guess this belongs to ConfigurationType* getConfiguration(); from above

NicolasFussberger · 2026-07-06T09:46:23Z

+        new_config["initial_run_target"] = config["initial_run_target"]
+
+    if "fallback_run_target" in config:
+        fallback_defaults = {


what was the reason for not having this in the default dictionary at the top?

NicolasFussberger · 2026-07-06T09:50:52Z

+        deployment = {
+            "ready_timeout": depl_cfg["ready_timeout"],
+            "shutdown_timeout": depl_cfg["shutdown_timeout"],
+            "bin_dir": depl_cfg.get("bin_dir", "/opt"),


There are already defaults for bin_dir and working_dir.
I wonder if we should avoid repeating this here.

NicolasFussberger · 2026-07-06T09:55:37Z

+    }
+
+    watchdog_config = config.get("watchdog", {})
+    if watchdog_config and "device_file_path" in watchdog_config:


I wonder why checking specifically for device_file_path?
Ideally, device_file_path and max_timeout should be mandatory in the schema if a watchdog is defined at all. But currently, it is not.

NicolasFussberger · 2026-07-06T09:56:33Z

+
+
+def output_filename(input_filename):
+    return "launch_manager_config_gen.json"


Can we make this depending on the input filename?

NicolasFussberger · 2026-07-06T10:00:38Z

    schema = ctx.file.schema
+    script = ctx.executable.script
+    json_out_dir = ctx.attr.json_out_dir
+    use_new_format = ctx.attr.use_new_format


Would it be possible to simplify the variant handling here to just always generate all the configs?
The naming should never conflict as we have fixed names for the old config.

Then we are able to have the test cases for the config generation valid for both old and new config until the old config is removed

paulquiring requested a deployment to workflow-approval June 15, 2026 14:18 — with GitHub Actions Waiting

paulquiring requested a deployment to workflow-approval June 16, 2026 06:42 — with GitHub Actions Waiting

paulquiring commented Jun 16, 2026

View reviewed changes

Comment thread scripts/config_mapping/lifecycle_config.py

paulquiring marked this pull request as ready for review June 16, 2026 08:02

paulquiring requested a review from NicolasFussberger June 16, 2026 08:02