refactor: Remove InterpretStatus/Error & Outcome from Topology #99

letian · 2025-08-07T02:42:23Z

letian commented

2025-08-07 02:42:23 +00:00

The InterpretStatus, InterpretError, and Outcome leaked in places it shouldn't have: the Topology. Because of this, refactoring to better track & instrument Interpret was more difficult as it might have effects on the Topology as well. Which wasn't a desirable outcome.

This PR introduces a TopologyState along with its TopologyStatus, and a PreparationOutcome along with its PreparationError.

A clear separation was introduced between the TopologyState/TopologyStatus and the PreparationOutcome/PrepareError because they serve different purposes/were needed by different actors.

TopologyState/TopologyStatus serves only the Maestro to track the Topology state (whether it's ready or not). Whereas the PreparationOutcome/PreparationError serves the Topology itself for every individual actions it might take in order to be ready (e.g. installing k8s client, configuring the k8s tenant manager, installing prometheus, etc.).

For now, I couldn't find an elegant way to move the TopologyState management inside the topology itself without pushing this responsibility back to the implementers of each Topology. So that's why it's currently handled by the Maestro itself. Which in a way makes sense as it's part of its responsibilities.

Sidequest (sorry 😅): also replace the multiple Topology events by a single TopologyStateChanged event. We will probably do the same for the Interpret events later on.

The `InterpretStatus`, `InterpretError`, and `Outcome` leaked in places it shouldn't have: the `Topology`. Because of this, refactoring to better track & instrument `Interpret` was more difficult as it might have effects on the `Topology` as well. Which wasn't a desirable outcome. This PR introduces a `TopologyState` along with its `TopologyStatus`, and a `PreparationOutcome` along with its `PreparationError`. A clear separation was introduced between the `TopologyState`/`TopologyStatus` and the `PreparationOutcome`/`PrepareError` because they serve different purposes/were needed by different actors. `TopologyState`/`TopologyStatus` serves only the `Maestro` to track the `Topology` state (whether it's ready or not). Whereas the `PreparationOutcome`/`PreparationError` serves the `Topology` itself for every individual actions it might take in order to be ready (e.g. installing k8s client, configuring the k8s tenant manager, installing prometheus, etc.). For now, I couldn't find an elegant way to move the `TopologyState` management inside the topology itself without pushing this responsibility back to the implementers of each `Topology`. So that's why it's currently handled by the `Maestro` itself. Which in a way makes sense as it's part of its responsibilities. --- Sidequest (sorry 😅): also replace the multiple Topology events by a single `TopologyStateChanged` event. We will probably do the same for the Interpret events later on.

letian added 1 commit 2025-08-07 02:42:26 +00:00

refactor: Remove InterpretStatus/Error & Outcome from Topology

Run Check Script / check (pull_request) Has been cancelled

Details

f876b5e67b

johnride approved these changes 2025-08-08 17:13:15 +00:00

johnride left a comment

LGTM 🎉

harmony/src/domain/topology/mod.rs

						
				@ -63,0 +73,4 @@

				}

				#[derive(Debug, Clone, new)]

				pub struct PreparationError {

johnride commented

2025-08-08 17:13:08 +00:00

I love this! Feels natural and clean.

Also very naturally outlines a trend to eventually avoid the boilerplate of having each module define its own set of Outcome, Error. We will then refactor with a standardized approach when time comes.

I love this! Feels natural and clean. Also very naturally outlines a trend to eventually avoid the boilerplate of having each module define its own set of <X>Outcome, <X>Error. We will then refactor with a standardized approach when time comes.

👍 1