Read the operational mode from Python components - #99
Conversation
Bump the component-graph dependency from v0.6.0 to v0.6.1, which adds an operational mode to graph components: one that provides no telemetry is not used as a measurement source. Reading that mode from Python components follows in the next commit. Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
Translate the operational mode the way the category is translated: Python splits the mode into `provides_telemetry()` and `accepts_control()`, and the two together name one `OperationalMode`. A mode is named only when both flags are known -- `provides_telemetry() == false` fits both `Inactive` and `ControlOnly` -- and a component may lack the methods entirely, as the assets client does. Both cases read as unspecified rather than failing the graph. Raise the `microgrid` extra to `frequenz-client-microgrid >= 0.18.4`, the release the two methods arrived in. On the old floor of 0.18.3 the mode was never readable and the mode tests could not run. Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
Bump the crate version to 0.5.1 and write the release summary. Drop the empty Bug Fixes section, as CONTRIBUTING asks for at release time. Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
github-actions
Bot
added
part:docs
| monkeypatch.delattr(Component, "provides_telemetry") | ||
| monkeypatch.delattr(Component, "accepts_control") |
There was a problem hiding this comment.
Why are you testing this works specifically? Are you doing this to support older versions for client-microgrid? Otherwise those attributes should be always present. I don't get this.
There was a problem hiding this comment.
Testing this case, so that I can make a patch release of this package. Then older SDK can also pick it up without issues.
| unspecified mode. It is treated as providing telemetry, so graphs that | ||
| never set a mode keep their formulas. | ||
| """ | ||
| graph = _pv_graph_with_modes(provides_telemetry=None, accepts_control=None) |
There was a problem hiding this comment.
Small warning. I changed this in client-common to use bool | int instead of bool | None so we can also represent a new enum value (I think it will be extremely unlikely that we get one, but still, to match the style we use for other enums), and 0 represents unspecified, so this code will need a small update when moving to client-common (but I guess it will require many more updates anyway).
|
|
||
| fn operational_mode(&self) -> cg::OperationalMode { | ||
| self.operational_mode | ||
| } | ||
| } |
There was a problem hiding this comment.
Wouldn't it make more sense to match the python interface here and use provides_telemetry and accepts_control instead? Or will this be a too intrusive change for the underlying rust code? The idea of splitting the enum in 2 properties was making use simpler, so if you are then merging it back the split seems to be useless.
2 participants
frequenz-microgrid-component-graphv0.6.1 adds an operational mode to graphcomponents, but the trait default leaves every component
Unspecified— thebindings have to read the mode off each Python component for it to have any
effect. This builds against v0.6.1, translates the mode, and releases 0.5.1.
Changes
provides_telemetry()/accepts_control()into oneOperationalModeinComponent::try_new, alongside the existing categorytranslation.
microgridextra's floor tofrequenz-client-microgrid >= 0.18.4,the release the two methods arrived in. On 0.18.3 the mode is never readable
and the new tests cannot run.
Worth a look
provides_telemetry() == falsefitsInactiveandControlOnlyalike, so ahalf-known mode stays
Unspecifiedrather than being guessed at.methods, so their absence reads as unspecified instead of failing the graph —
the same tolerance the category lookup already has.
getattrandcall0are separate steps so a missing method reads asunspecified while an
AttributeErrorfrom inside the method body propagates.call_method0cannot tell those apart, and swallowing the second would hide acaller's bug and leave the component measuring.