You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
After #241, InvalidDeliveryArea.__str__ uses an explicit <invalid:VALUE> marker to flag invariant-violating fields (empty code, code_type=0). The same convention is being adopted in #248 (InvalidLatitude/Longitude/CountryCode, tracked via review comment r3589470688).
Several other wrapper types across the library carry malformed / unspecified / unrecognized data but don't yet follow this convention. They should be updated so grep '<invalid:' on logs finds every invariant violation regardless of which type produced it.
Rationale for the <invalid:VALUE> marker (over ! prefix/suffix, ❌, class-name-only, or bare repr()): ASCII + greppable, composes per field, no collision with legitimate content. Full write-up at r3589470688 and reference implementation at commit 5688575.
Proposed solution
Existing wrapper types in this repo (no active PR):
Class
File
Current
Needs
MetricConnection.__str__
src/frequenz/client/common/metrics/_sample.py
Renders raw int bare for unspecified/unknown category, though get_category() raises on 0 | MetricConnectionCategory.UNSPECIFIED. The existing wrapping on the known case uses <CATEGORY=NAME> (uppercase =), inconsistent with the new <invalid:VALUE> (lowercase :).
Render <invalid:0> for 0 | MetricConnectionCategory.UNSPECIFIED to match get_category(); normalize (or drop) the <CATEGORY=NAME> wrapping on the known case.
InvalidLifetime (Improve Lifetime validation and error handling #249err-lifetime branch) — needs a design decision. Unlike the per-field violations above, InvalidLifetime's invariant is relational (start > end), not per-field. Suggested: add a __str__ on BaseLifetime rendering [{start_time}, {end_time}], override on InvalidLifetime to wrap the whole rendering: <invalid:[{start_time}, {end_time}]>.
Tests for the classes above need updating to match the new __str__ outputs.
Use cases
Log triage: grep '<invalid:' across logs finds every invariant violation regardless of type. Currently readers need to know each wrapper's convention (or that a bare 0 in MetricConnection[0] means "unspecified" rather than "category-0").
Consistency for downstream users: application code that formats or logs any of these types sees the same shape of invalid marker regardless of what produced it.
Alternatives and workarounds
Considered ! prefix/suffix, ❌ emoji, class-name-only marker, and bare repr() fallback. Full comparison and rationale: PR #248 r3589470688.
Additional context
Reference implementation: 5688575 on name-delivery-area — InvalidDeliveryArea.__str__ overrides with per-field <invalid:VALUE> markers; DeliveryArea keeps a simple __str__ (invariants trusted on the valid class).
What's needed?
After #241,
InvalidDeliveryArea.__str__uses an explicit<invalid:VALUE>marker to flag invariant-violating fields (emptycode,code_type=0). The same convention is being adopted in #248 (InvalidLatitude/Longitude/CountryCode, tracked via review comment r3589470688).Several other wrapper types across the library carry malformed / unspecified / unrecognized data but don't yet follow this convention. They should be updated so
grep '<invalid:'on logs finds every invariant violation regardless of which type produced it.Rationale for the
<invalid:VALUE>marker (over!prefix/suffix,❌, class-name-only, or barerepr()): ASCII + greppable, composes per field, no collision with legitimate content. Full write-up at r3589470688 and reference implementation at commit5688575.Proposed solution
Existing wrapper types in this repo (no active PR):
MetricConnection.__str__src/frequenz/client/common/metrics/_sample.pyintbare for unspecified/unknowncategory, thoughget_category()raises on0 | MetricConnectionCategory.UNSPECIFIED. The existing wrapping on the known case uses<CATEGORY=NAME>(uppercase=), inconsistent with the new<invalid:VALUE>(lowercase:).<invalid:0>for0 | MetricConnectionCategory.UNSPECIFIEDto matchget_category(); normalize (or drop) the<CATEGORY=NAME>wrapping on the known case.UnrecognizedElectricalComponentsrc/frequenz/client/common/microgrid/electrical_components/_problematic.pycategory: intfrom the wire; not visible in the inheritedElectricalComponent.__str__(only the class name signals the problem).__str__to expose the raw value as<invalid:{category}>.MismatchedCategoryElectricalComponentsrc/frequenz/client/common/microgrid/electrical_components/_problematic.pyUnrecognizedBatterysrc/frequenz/client/common/microgrid/electrical_components/_battery.pytype: int; not visible in inherited__str__.__str__to expose the raw value as<invalid:{type}>.UnrecognizedEvChargersrc/frequenz/client/common/microgrid/electrical_components/_ev_charger.pyUnrecognizedInvertersrc/frequenz/client/common/microgrid/electrical_components/_inverter.pyIn-flight PR needing alignment:
InvalidLifetime(ImproveLifetimevalidation and error handling #249err-lifetimebranch) — needs a design decision. Unlike the per-field violations above,InvalidLifetime's invariant is relational (start > end), not per-field. Suggested: add a__str__onBaseLifetimerendering[{start_time}, {end_time}], override onInvalidLifetimeto wrap the whole rendering:<invalid:[{start_time}, {end_time}]>.Tests for the classes above need updating to match the new
__str__outputs.Use cases
grep '<invalid:'across logs finds every invariant violation regardless of type. Currently readers need to know each wrapper's convention (or that a bare0inMetricConnection[0]means "unspecified" rather than "category-0").Alternatives and workarounds
Considered
!prefix/suffix,❌emoji, class-name-only marker, and barerepr()fallback. Full comparison and rationale: PR #248 r3589470688.Additional context
5688575onname-delivery-area—InvalidDeliveryArea.__str__overrides with per-field<invalid:VALUE>markers;DeliveryAreakeeps a simple__str__(invariants trusted on the valid class).DeliveryAreavalidation and error handling #241 (grid, landed), ImproveLocationvalidation and error handling #248 (types/location, tracked via review comment).