FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Improve validation by llucax · Pull Request #217 · frequenz-floss/frequenz-client-common-python · GitHub

Repository navigation

Improve validation - #217

Merged
llucax merged 3 commits into
frequenz-floss:v0.x.xfrom
llucax:validation
Jun 18, 2026
Merged

llucax merged 3 commits into
frequenz-floss:v0.x.xfrom
llucax:validation

Conversation

llucax commented Jun 16, 2026

Copy link
Copy Markdown
Contributor
  • Validate Location coordinates in __post_init__
  • Validate PaginationInfo.total_items is non-negative
  • Fix rated_fuse_current validation error message

Part of #203.

llucax requested a review from a team as a code owner June 16, 2026 11:01
llucax requested review from shsms and removed request for a team June 16, 2026 11:01
llucax self-assigned this Jun 16, 2026
llucax added this to the v0.4.1 milestone Jun 16, 2026
github-actions Bot added part:tests Affects the unit, integration and performance (benchmarks) tests part:microgrid Affects the microgrid protobuf definitions part:pagination Affects the pagination protobuf definitions labels Jun 16, 2026
llucax added cmd:skip-release-notes It is not necessary to update release notes for this PR and removed part:tests Affects the unit, integration and performance (benchmarks) tests part:microgrid Affects the microgrid protobuf definitions part:pagination Affects the pagination protobuf definitions labels Jun 16, 2026
llucax enabled auto-merge June 16, 2026 11:02
llucax added part:tests Affects the unit, integration and performance (benchmarks) tests part:microgrid Affects the microgrid protobuf definitions part:pagination Affects the pagination protobuf definitions labels Jun 16, 2026
llucax added 3 commits June 18, 2026 14:26
Reject out-of-range and NaN latitude or longitude values on direct
construction using the same inclusive bounds as the proto converter,
while keeping converter sanitization unchanged.

Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
Reject negative `total_items` values with the exact value in the error
message while keeping zero and positive values valid.

Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
Keep accepting zero by preserving the `< 0` guard and correct the
message to say non-negative instead of positive.

Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

llucax commented Jun 18, 2026

Copy link
Copy Markdown
Contributor Author

Ping.

llucax added this pull request to the merge queue Jun 18, 2026
Merged via the queue into frequenz-floss:v0.x.x with commit dd3ec03 Jun 18, 2026
9 checks passed
llucax deleted the validation branch June 18, 2026 12:50
llucax added a commit to llucax/frequenz-client-common-python that referenced this pull request Sep 16, 2026
…nz-floss#285)

`pagination_info_from_proto()` calls the `PaginationInfo` constructor
directly, so the `ValueError` that frequenz-floss#217 added for a negative
`total_items` can escape from a conversion function. That breaks the
wrapping rule that loading from the wire never raises on invalid
content: the caller loses the whole message over one bad field.

This adds the usual valid/invalid pair and the replacement converter:

* `BasePaginationInfo` holds the two shared fields and refuses direct
instantiation.
* `PaginationInfo` keeps its `ValueError` unchanged. There is
deliberately no `_raise_on_invalid`-style escape hatch: unlike
`DeliveryArea`, it has always been strict here, so there is no released
behavior to transition away from.
* `InvalidPaginationInfo` takes the same fields with no invariants, so a
malformed count survives for inspection.
* `pagination_info_from_proto2()` returns `PaginationInfo |
InvalidPaginationInfo`; `pagination_info_from_proto()` is deprecated and
points at it, mirroring `delivery_area_from_proto` and
`bounds_from_proto`.

Two things worth a second look while reviewing:

The new converter reads `next_page_token` through `HasField()` instead
of testing it for truth. The field has presence, so the old truthiness
test collapses "the server sent an empty token" into "the server sent no
token", and only the second means there are no more pages. Changing that
in place would be a silent behavior change, so it only happens under the
new name.

`total_items` is a `uint32` on the wire, so a decoded message cannot
actually carry a negative count today and the invalid branch is
unreachable through the generated bindings. It is still there because
the converter's contract is not to raise regardless of what it is
handed, and because the wire type is not the wrapper's to assume; the
test exercises it with a message-like stand-in.

Both leaves also gain a `__str__`, which neither had. The pair only
reads well if the invalid marker composes with a valid form, and the
dataclass `repr` is too long for a log line. The rule is per-field, so
the marker goes on the count (`items=<invalid:-1>,next=None`) rather
than around the whole object, as in `InvalidDeliveryArea`.

The release notes document the deprecation and, separately, the breaking
change from frequenz-floss#217: `PaginationInfo(total_items=-1)` raised nothing in
v0.4.0 and raises `ValueError` now.

Part of frequenz-floss#239 and frequenz-floss#251.
llucax added a commit to llucax/frequenz-client-common-python that referenced this pull request Sep 16, 2026
Fill the gaps found in a pre-release sanity check of `RELEASE_NOTES.md`:

* Replace the template placeholder with a real release summary,
including release stats and the note that this is a huge release despite
the patch version, realizing the new library design while keeping
backwards compatibility, with the breaking cleanup to follow in v0.5.0.
* Document two breaking changes to symbols released in v0.4.0 that were
missing from Upgrading: `Bounds` raising on `NaN` endpoints (frequenz-floss#254) and
`MetricConnection.name` no longer being optional (frequenz-floss#257).
* Complete the `UNSPECIFIED`-as-`0` bullet with `metric_config_bounds`
keys (frequenz-floss#224), merge the duplicated `Microgrid` bullets, and briefly cover
the `ElectricalComponent` accessors, `CategorySpecificInfo` and the
smaller `Invalid*` types from frequenz-floss#237, frequenz-floss#248, frequenz-floss#249, frequenz-floss#257, frequenz-floss#268 and frequenz-floss#273.
* Add the `str()` exception message change (frequenz-floss#258) to Bug Fixes.

The `PaginationInfo` breaking change from frequenz-floss#217 is documented separately
in frequenz-floss#285, which adds `InvalidPaginationInfo` and
`pagination_info_from_proto2`.
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

Labels

cmd:skip-release-notes It is not necessary to update release notes for this PR part:microgrid Affects the microgrid protobuf definitions part:pagination Affects the pagination protobuf definitions part:tests Affects the unit, integration and performance (benchmarks) tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants


Back | FazBrowse Home | New Git URL