Improve Location validation and error handling#248
Draft
llucax wants to merge 9 commits into
Draft
Conversation
llucax
added
the
cmd:skip-release-notes
github-actions
Bot
added
part:docs
llucax
force-pushed
the
err-location
branch
2 times, most recently
from
cefacfc to
27f666a
Compare
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>
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>
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>
Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
Add three new subclasses of `InvalidAttributeError`: * `InvalidLatitudeError` — the raw `float` was outside `[-90, 90]` * `InvalidLongitudeError` — the raw `float` was outside `[-180, 180]` * `InvalidCountryCodeError` — the raw `str` was not exactly 2 characters Each stores the offending value on `.value` (`float` for the coordinate errors, `str` for the country code error). These will be raised by the upcoming `Location.get_latitude()`, `.get_longitude()` and `.get_country_code()` accessors (next commits), where the low-level fields on `Location` may carry the raw invalid data that came off the wire. Also split `test_location.py` into multiple files as it will grow. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
These classes will be used to store invalid location attributes explicitly, so they can still be inspected, but can't be used accidentally. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
Use the new invalid types for typing `Location` attributes, so invalid values are only accepted while using the wrapper type. This avoids accidental usage of invalid values, and accidental construction of invalid locations, while still allowing for inspection and construction when invalid values are explicitly requested. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
Add `get_latitude()`, `get_longitude()`, `get_country_code()` and `get_country_code_or_none()`. These accessors raise on invalid (or sometimes missing) data. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
Add `Microgrid.get_location() -> Location`, the safe counterpart of the
`Microgrid.location` field. It resolves the optional underlying field to
a concrete `Location`:
* `None` (the field was not set on the wire) → `MissingFieldError`.
* `Location` → returned unchanged.
Callers that need validated coordinates or a validated country code
should chain through the new
`Location.get_{latitude,longitude,country_code}()` accessors on the
returned instance.
Also generalize the file-local `_make_microgrid` test helper to accept
both `delivery_area` and `location` as keyword-only arguments (both
default to `None`), and update the existing `get_delivery_area()` tests
to pass `delivery_area` by keyword — no behavior change, just the same
helper covering both accessor test suites.
Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
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.
Make
latitudeandlongituderequired to match the protobuf definition while keepingcountry_codeoptional even when it is required because in practice it could be missing (encoded as"").We now also allow storing invalid values, but encoded via new types:
InvalidLatitude,InvalidLongitude, andInvalidCountryCode. This way users still can't read invalid values accidentally. This allows users to inspect invalid values if they need to, instead of only reporting via logging.Add new safe accesors for
Locationattributes andMicrogrid.locationto allow users to safely access the values without having to check for validity themselves.Part of #239.