Improve DeliveryArea validation and error handling#241
Open
llucax wants to merge 14 commits into
Open
Conversation
github-actions
Bot
added
part:docs
5 tasks
llucax
force-pushed
the
name-delivery-area
branch
2 times, most recently
from
9e9a1c6 to
8e6ce9e
Compare
github-actions
Bot
added
part:tooling
Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
`UnrecognizedEnumValueError` should only be raised from high-level wrappers, not protobuf conversion functions. Replace it with a simple `ValueError`, in this case someone just called the function with the wrong arguments. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
This exception will be the base for any exception raised when an attribute that holds invalid data is accessed through a semantic accessor. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
Add a new top-level exception `MissingFieldError`, sibling of `UnspecifiedEnumValueError`, to signal that a semantic accessor was called on a wrapper field that resolves to `None` because the underlying field was not set on the wire. The first user is the upcoming `Microgrid.get_delivery_area()` accessor (delivery area may be `None`, `InvalidDeliveryArea`, or `DeliveryArea`), but the pattern is expected to recur wherever an accessor unifies a `T | ... | None` field into a plain `T` return. Like `Unspecified/UnrecognizedEnumValueError`, it multi-inherits from `ClientCommonError` and `ValueError` for convenience. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
Make `UnrecognizedEnumValueError` and `UnspecifiedEnumValueError` inherit from `InvalidAttributeError`. Also split exception tests as the file grew too much when adding the new tests. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
This class will be used as the base for both valid and invalid delivery area classes. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
`DeliveryArea`, the existing type, is retroactively made a subclass of `BaseDeliveryArea`. Field shape and construction API are unchanged (`code: str | None`, `code_type: EnergyMarketCodeType | int`) for backwards compatibility, but a `__post_init__` is added with invariant checks: a well-formed delivery area must have a non-empty `code` and a specified `code_type`. Constructing one without that doesn't pass those invariants will now emit a `DeprecationWarning` (and it will raise `ValueError` in v0.5.0). Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
A `__post_init__` is added with invariant checks: a well-formed delivery area must have a non-empty `code` and a specified `code_type`. Constructing one without that doesn't pass the invariant will now emit a `DeprecationWarning` (and it will raise `ValueError` in v0.5.0). Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
`InvalidDeliveryArea` is added to represent malformed wire data. Same fields as `BaseDeliveryArea`, no invariants enforced, so callers can inspect whatever the server actually sent. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
The dedicated `InvalidDeliveryAreaError` (also a `ValueError` for convenience) carries the offending `InvalidDeliveryArea` on `.delivery_area` and will be raised by upcoming semantic accessors (next commits) instead of forcing callers to distinguish the two. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
Introduce `delivery_area_from_proto2`, sibling of the existing `delivery_area_from_proto`, returning `DeliveryArea | InvalidDeliveryArea` instead of a plain `DeliveryArea`. The new converter enforces at the boundary the invariant that `DeliveryArea.__post_init__` now warns about: * `code` must be non-empty. * `code_type` must not be raw `0` (i.e. unspecified). A wire message that fails either check becomes an `InvalidDeliveryArea` carrying the raw data verbatim, so callers can inspect or report it via `InvalidDeliveryAreaError`. Unknown non-zero `int` `code_type` values are still treated as valid to preserve forward compatibility with new protobuf enum values. Because the valid path constructs a `DeliveryArea` only when the invariants already hold, `delivery_area_from_proto2` never triggers the inner `DeprecationWarning` — and, unlike the old converter, it doesn't log any "found issues" warning either: the return type itself now signals whether the wire data was well-formed. The existing `delivery_area_from_proto` remains unchanged and will be marked `@deprecated` in a follow-up commit. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
Loosen `Microgrid.delivery_area` from `DeliveryArea | None` to `DeliveryArea | InvalidDeliveryArea | None` to reflect the three possible wire states: absent, present-but-malformed, and well-formed. `microgrid_from_proto` is switched to `delivery_area_from_proto2`, so proto-loaded microgrids already carry the right variant on the union. Add the semantic accessor `Microgrid.get_delivery_area() -> DeliveryArea` so callers that need a valid delivery area don't have to check for either failure mode by hand: * `None` -> `MissingFieldError` (the field wasn't set on the wire). * `InvalidDeliveryArea` -> `InvalidDeliveryAreaError`, with the offending instance available on `.delivery_area` for inspection. * `DeliveryArea` -> returned unchanged. Microgrid is unreleased so this widened field and the new accessor are free additions — no backwards compatibility needed. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
Mark `delivery_area_from_proto` as `@deprecated`, pointing callers at `delivery_area_from_proto2`, which returns `DeliveryArea | InvalidDeliveryArea` and surfaces malformed wire data at the type level rather than as an ad-hoc log warning. The internal `DeliveryArea(...)` construction inside `delivery_area_from_proto` was already wrapped in `warnings.catch_warnings()` when `DeliveryArea.__post_init__` gained its soft invariant check, so the two suppressions compose correctly: callers of the deprecated entry point see exactly one `DeprecationWarning`, identifying `delivery_area_from_proto2` as the replacement. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
llucax
force-pushed
the
name-delivery-area
branch
from
8e6ce9e to
8a24201
Compare
llucax
requested review from
eduardiazf and
ela-kotulska-frequenz
and removed request for
a team
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a new delivery-area class hierarchy:
frequenz.client.common.grid.BaseDeliveryArea— abstract common supertype of the two concrete leaves; not directly instantiable.frequenz.client.common.grid.DeliveryArea— well-formed delivery area (retroactively made a subclass ofBaseDeliveryArea; its shared__str__andget_code_type()are hoisted to the base).frequenz.client.common.grid.InvalidDeliveryArea— malformed wire data; same fields asDeliveryAreawith no invariants enforced, so callers can inspect whatever the server actually sent.Also adds
frequenz.client.common.grid.proto.v1alpha8.delivery_area_from_proto2returningDeliveryArea | InvalidDeliveryArea. This is the replacement fordelivery_area_from_proto(which is deprecated now) and is whatmicrogrid_from_protouses internally.This deprecates construction of
frequenz.client.common.grid.DeliveryAreawith invalid data. A well-formedDeliveryAreahas a non-emptycodeand a non-UNSPECIFIEDcode_type. Constructing one with invalid data currently emits aDeprecationWarning; a future release will replace the warning with a hardValueError. Preferdelivery_area_from_proto2to load delivery areas from the wire — malformed messages becomeInvalidDeliveryAreainstances instead.Part of #239.