Removing Config Adapter by MaciejKaszynski · Pull Request #455 · eclipse-score/lifecycle

MaciejKaszynski · 2026-08-12T08:17:24Z

Fixes #420

Currently we have a lot of code to bridge the old config and new. This PR removes the bridge and changes. Process Group Manager, Graph and Alive Monitor to use the new config.

This also partially addresses some multi-og code from #413. However our IPC still has a field for the pg_name and there is still a bunch of references to it in comments and method names.

This also creates a temporary CreateDependencyGraph function that takes the config and creates a DependencyGraph from it. This is intentionally left as not polished and un covered with UTs as it shall be more or less removed in #463.

Another point to address is that the AliveMonitor copies its config as the shape of the config doesn't fit the current implementation. This will be addressed in #477.

github-actions · 2026-08-13T07:14:54Z

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: 7fb2072a-30a1-4de3-bc91-c686983947c3
Computing main repo mapping: 
Computing main repo mapping: 
Loading: 
Loading: 0 packages loaded
Loading: 0 packages loaded
Loading: 0 packages loaded
Loading: 0 packages loaded
    currently loading: 
Loading: 0 packages loaded
    currently loading: 
Loading: 0 packages loaded
    currently loading: 
Loading: 0 packages loaded
    currently loading: 
Loading: 0 packages loaded
    currently loading: 
Loading: 0 packages loaded
    currently loading: 
Analyzing: target //:license-check (1 packages loaded, 0 targets configured)
Analyzing: target //:license-check (1 packages loaded, 0 targets configured)

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

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

Analyzing: target //:license-check (146 packages loaded, 2945 targets configured)

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

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

Analyzing: target //:license-check (172 packages loaded, 9451 targets configured)

Analyzing: target //:license-check (172 packages loaded, 9451 targets configured)

Analyzing: target //:license-check (174 packages loaded, 9575 targets configured)

INFO: Analyzed target //:license-check (177 packages loaded, 11589 targets configured).
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: 23.740s, Critical Path: 0.25s
INFO: 16 processes: 4 disk cache hit, 12 internal.
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>]

github-actions · 2026-08-14T14:32:22Z

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

danth

The graph creation function is a bit long, but since it will be improved anyway after #463, I don't think it is worth making any major changes in this pull request.

danth · 2026-08-14T15:00:53Z

 {
    score::concurrency::InterruptibleFuture<void> retVal_{};

+    std::cout << "Setting state " << pg_name << pg_state << std::endl;


Use LM_LOG_DEBUG, or remove this.

danth · 2026-08-14T15:20:27Z

+    ISupervisionEventPublisher& supervision_event_publisher,
+    osal::IProcess* process_interface,
+    std::shared_ptr<SafeProcessMapInserter> process_map,
+    std::unordered_map<std::size_t, GraphIndex>& run_target_map)


run_target_map is not documented

danth · 2026-08-14T15:21:22Z


-    LM_LOG_DEBUG() << "Process group index" << index << "(with name" << pg_name << ") has" << num_processes
-                   << "processes";
+    DependencyGraph<Graph::Component> graph(components.size() + run_targets.size() + 1);


Should we add a comment explaining why the + 1 is necessary?

danth · 2026-08-14T15:22:33Z

+        component_name_to_index[component_name] = index;
+
+        // all of this relies on graphindex having sequential indexes
+        // TODO use IdHash later...


Maybe add a link to #463 and explain that this method will be simplified after that change.

danth · 2026-08-14T15:23:16Z

        {
-            return static_cast<int32_t>(index);
+            auto it = component_name_to_index.find(dep_name);
+            LM_LOG_ERROR() << run_targets[i].name << "HAS DEP TO " << dep_name;


Suggested change

LM_LOG_ERROR() << run_targets[i].name << "HAS DEP TO " << dep_name;

LM_LOG_DEBUG() << run_targets[i].name << "HAS DEP TO " << dep_name;

danth · 2026-08-14T15:27:47Z

+#include <ios>
+#include <iostream>


Not needed.

danth · 2026-08-14T15:29:18Z

-    // Wait for process group states to change while actively draining shutdown events.
+    if (!graph_)
+    {
+        return;


Should we log a warning here?

Think I will actually make all graph_ checks to asserts as this shall never happen. We could actually make this not a shared_ptr and then remove the checks at all, but I will make an issue for this to do in another PR

danth · 2026-08-14T15:30:23Z

+        LM_LOG_DEBUG() << "HERER++++++++";
        // Now process the request
        switch (scc->request().request_or_response_)
        {
            case ControlClientCode::kSetStateRequest:
+                LM_LOG_DEBUG() << "HERER++++++++";


Remove print debugging.

danth · 2026-08-14T15:31:52Z

+            // TODO: Determine recovery state from configuration
+            recovery_state.pg_state_name_ = IdentifierHash("fallback");  // TODO: Get from configuration


Does this need to be done?

danth · 2026-08-14T15:36:31Z

+            SCORE_LANGUAGE_FUTURECPP_PRECONDITION_MESSAGE(
+                it != component_name_to_index.end(), "RunTarget dependency not found in component list");
+
+            graph.addDependency(num_of_components + i, it->second);


I think this would be clearer if we had something like

for (std::size_t run_target_i = 0; run_target_i < run_targets.size(); ++run_target_i) { std::size_t node_i = num_of_components + run_target_i;

WilliamRoebuck

Looks good, I have mostly small comments, one or two suggestions for code changes

WilliamRoebuck · 2026-08-17T10:47:41Z

 #include <sys/stat.h>
 #include <cerrno>
 #include <cstdint>
+#include <iostream>


WilliamRoebuck · 2026-08-17T11:00:07Z

    EXPECT_THAT(target.recovery_action.run_target, Eq("SafeState"));
 }

+TEST_F(FlatbufferConfigLoaderTest, ConfiguredOffRunTargetIsLoadedVerbatim)


Can we add or extend a test to check that an "Off" target is created if none exists?

WilliamRoebuck · 2026-08-17T11:06:13Z

+using namespace score::mw::lifecycle::internal;
+namespace configuration = score::mw::lifecycle::internal::configuration;


These are probably not needed anymore

WilliamRoebuck · 2026-08-17T11:09:31Z

 class Graph final
 {
  public:
+    /// @brief All possible implementation of a Component.


Suggested change

/// @brief All possible implementation of a Component.

/// @brief All currently supported component implementations

WilliamRoebuck · 2026-08-17T11:57:24Z

+            auto it = component_name_to_index.find(dep_name);
+            SCORE_LANGUAGE_FUTURECPP_PRECONDITION_MESSAGE(
+                it != component_name_to_index.end(), "Component dependency not found in component list");

-    SCORE_LANGUAGE_FUTURECPP_ASSERT_DBG_MESSAGE(states != nullptr, "Process group states not found for process group");
+            graph.addDependency(comp_dep_i, it->second);


This addDependency section is repeated 3 times, can we add a helper function/lambda? Or even better maybe we can iterate through all the dependencies in one loop somehow?

Maybe you can add everything to the graph and then iterate through graph items, accessing the config by index?

WilliamRoebuck · 2026-08-17T13:02:19Z

 /// @details The implementation should be async signal safe.
-OsalReturnType ProcessLauncher::setSchedulingAndSecurity(const OsalConfig& config)
+OsalReturnType ProcessLauncher::setSchedulingAndSecurity(
+    const score::mw::lifecycle::internal::configuration::ComponentConfig& config)


Can we pass just the deployment config here?

WilliamRoebuck · 2026-08-17T13:12:56Z

    alive_monitor_thread_->stop();
-    configuration_.deinitialize();
-    process_groups_.clear();
+    // No deinitialize needed - Config destructor handles cleanup


I don't think we need this comment

WilliamRoebuck · 2026-08-17T13:36:02Z

-    }
-    return result;
+    SCORE_LANGUAGE_FUTURECPP_ASSERT_MESSAGE(bool(graph_), "Graph not initialized");
+    // Convert string_view to IdentifierHash


There's a constructor at IdentifierHash.cpp:66, doesn't it work?

WilliamRoebuck · 2026-08-17T13:39:16Z

+            // TODO: Determine recovery state from configuration
+            recovery_state.pg_state_name_ = IdentifierHash("fallback");  // TODO: Get from configuration


Double // TODO: 🙂

WilliamRoebuck · 2026-08-17T13:39:46Z

+            recovery_state.pg_state_name_ = IdentifierHash("fallback");  // TODO: Get from configuration

-            LM_LOG_WARN() << "Problem discovered in PG" << recovery_state.pg_name_ << "Activating Recovery state.";
+            LM_LOG_WARN() << "Problem discovered, activating recovery state: " << recovery_state.pg_state_name_;


Suggested change

LM_LOG_WARN() << "Problem discovered, activating recovery state: " << recovery_state.pg_state_name_;

LM_LOG_WARN() << "Problem discovered, activating recovery state:" << recovery_state.pg_state_name_;

Extra space

github-project-automation Bot added this to LCM - Lifecycle & Health FT Aug 12, 2026

github-project-automation Bot moved this to Backlog in LCM - Lifecycle & Health FT Aug 12, 2026

MaciejKaszynski requested a deployment to workflow-approval August 12, 2026 08:17 — with GitHub Actions Waiting

MaciejKaszynski requested a deployment to workflow-approval August 13, 2026 07:13 — with GitHub Actions Waiting

MaciejKaszynski added 8 commits August 13, 2026 09:38

Inital changes

8118405

Rest is working

69ede3d

Fixing rebase

aa2482c

Graph converted

1b738e3

Fixing namespace

8420e6b

lm building

f2ea525

Fixing some tests

50ec974

Fix rebase

84721b9

MaciejKaszynski force-pushed the new-config branch from 48210c0 to 84721b9 Compare August 13, 2026 09:15

MaciejKaszynski requested a deployment to workflow-approval August 13, 2026 09:15 — with GitHub Actions Waiting

Fixing dep graph

7fff290

MaciejKaszynski requested a deployment to workflow-approval August 13, 2026 14:26 — with GitHub Actions Waiting

Some fixes

f961f8a

MaciejKaszynski requested a deployment to workflow-approval August 14, 2026 08:24 — with GitHub Actions Waiting

Adding bad fix

4bbbfed

MaciejKaszynski requested a deployment to workflow-approval August 14, 2026 08:28 — with GitHub Actions Waiting

Fixing last test

ee01564

MaciejKaszynski requested a deployment to workflow-approval August 14, 2026 10:51 — with GitHub Actions Waiting

MaciejKaszynski added 2 commits August 14, 2026 14:22

Moving watchdog

d87d3db

Fixing UTs

7a58b19

MaciejKaszynski deployed to workflow-approval August 14, 2026 13:45 — with GitHub Actions Active

Merge branch 'main' into new-config

98b3963

MaciejKaszynski requested a deployment to workflow-approval August 14, 2026 14:24 — with GitHub Actions Waiting

MaciejKaszynski marked this pull request as ready for review August 14, 2026 14:24

MaciejKaszynski requested review from FScholPer, NicolasFussberger, anmittag, antonkri, pawelrutkaq and ramceb as code owners August 14, 2026 14:24

danth reviewed Aug 14, 2026

View reviewed changes

Addressing some review comments

7920df4

MaciejKaszynski requested a deployment to workflow-approval August 17, 2026 10:44 — with GitHub Actions Waiting

WilliamRoebuck suggested changes Aug 17, 2026

View reviewed changes

	LM_LOG_ERROR() << run_targets[i].name << "HAS DEP TO " << dep_name;
	LM_LOG_DEBUG() << run_targets[i].name << "HAS DEP TO " << dep_name;

		// TODO: Determine recovery state from configuration
		recovery_state.pg_state_name_ = IdentifierHash("fallback"); // TODO: Get from configuration

		using namespace score::mw::lifecycle::internal;
		namespace configuration = score::mw::lifecycle::internal::configuration;

	/// @brief All possible implementation of a Component.
	/// @brief All currently supported component implementations

	LM_LOG_WARN() << "Problem discovered, activating recovery state: " << recovery_state.pg_state_name_;
	LM_LOG_WARN() << "Problem discovered, activating recovery state:" << recovery_state.pg_state_name_;

Uh oh!

Conversation

MaciejKaszynski commented Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Aug 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

License Check Results

Uh oh!

github-actions Bot commented Aug 14, 2026

Uh oh!

danth left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

WilliamRoebuck left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

MaciejKaszynski commented Aug 12, 2026 •

edited

Loading

github-actions Bot commented Aug 13, 2026 •

edited

Loading