Read the operational mode from Python components - #99
Conversation
github-actions
Bot
added
part:docs
| 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).
This adds support for using the operational mode of components in formula generation. It also includes a number of bug fixes. 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>
c4f1af9 to
13a5afc
Compare
|
I've also updated to component-graph-rs 0.6.2, which includes the bug fixes we merged last. |
llucax
left a comment
llucax
left a comment
There was a problem hiding this comment.
LGTM. I just don't get why is frequenz-client-microgrid a dependency at all, I don't see any imports for it in the repo.
| [project.optional-dependencies] | ||
| microgrid = [ | ||
| "frequenz-client-microgrid >= 0.18.3, < 0.19", | ||
| "frequenz-client-microgrid >= 0.18.4, < 0.19", |
There was a problem hiding this comment.
Unrelated, but is this really optional? Why? What is it used for?
There was a problem hiding this comment.
it is imported from the rust code, for checking the component category etc. Once it has moved to common, we'll just depend on common, and it becomes available for both assets and microgrid.
Right now it is optional because it is used in the tests, and it should be usable with just assets, without microgrid.
When we have common, we can make it a full dependency.
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.