Validate component IDs when creating component pools #1340
Conversation
Signed-off-by: Sahas Subramanian <[email protected]>
Signed-off-by: Sahas Subramanian <[email protected]>
Signed-off-by: Sahas Subramanian <[email protected]>
Signed-off-by: Sahas Subramanian <[email protected]>
Signed-off-by: Sahas Subramanian <[email protected]>
shsms
requested review from
ela-kotulska-frequenz
and removed request for
a team
github-actions
bot
added
part:docs
llucax
left a comment
llucax
left a comment
There was a problem hiding this comment.
Assigning to rc2300 because this is a breaking change. Also I think we need to mention it in the upgrading section of the release notes.
| if not self._batteries.issubset(all_batteries): | ||
| unknown_ids = self._batteries - all_batteries |
There was a problem hiding this comment.
You could save a line of code and a couple cycles by reversing this:
| if not self._batteries.issubset(all_batteries): | |
| unknown_ids = self._batteries - all_batteries | |
| if unknown_ids := self._batteries - all_batteries: |
(same for the rest)
There was a problem hiding this comment.
that doesn't look like a subset check.
| - `FormulaEngine` and `FormulaEngine3Phase` are now type aliases to `Formula` and `Formula3Phase`, fixing a typing issue introduced in `v1.0.0-rc2202`. | ||
| <!-- Here goes a general summary of what this release is about --> | ||
|
|
||
| ## Upgrading |
There was a problem hiding this comment.
I would also add a note here, as this is effectively a breaking change. Users will need to update their code to deal with the exception, or validate the IDs before passing them to new_xxx_pool().
llucax
added
the
scope:breaking-change
|
It is of course not a breaking change, because it was broken much worse earlier when they sent non-battery to battery pool. This is a more predictable meaningful error. Just a bug fix. |
2 participants
No description provided.