Validate component IDs when creating component pools #1340
Conversation
Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
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. |
|
It is a breaking change in the sense that raising a new exception is an interface change, but I see your point that passing the wrong ID would also make the user code explode, only in a different and worse way, so OK. Rolling-back the breaking change label and milestone. |
llucax
removed
the
scope:breaking-change
github-project-automation
bot
moved this from To do
to Review approved
in Python SDK Roadmap
|
Added a commit to add the missing Needs approval from someone else now. Enabling auto-merge too, as Sahas will be away for a few weeks. |
|
Tests still failing for the same reasons as #1343, see comments in that PR for details. |
Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
llucax
force-pushed
the
pool-input-validation
branch
from
26b8436 to
1c3574d
Compare
3 participants
No description provided.