fix: only decode fields used by the data class in Relation.load()#2636
Draft
joelnguemeta wants to merge 1 commit into
Draft
fix: only decode fields used by the data class in Relation.load()#2636joelnguemeta wants to merge 1 commit into
joelnguemeta wants to merge 1 commit into
Conversation
Juju automatically sets keys like 'ingress-address' on unit databags, with values that are not JSON-encoded. Relation.load() decoded every databag value before filtering by the class fields, so loading unit data into a dataclass or Pydantic model raised JSONDecodeError. Filter by the class fields first, and only decode matching values. Fixes canonical#2591
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
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.
Description
Relation.load()decodes every value in the databag before filtering by the class fields. Unit databags always contain keys automatically set by Juju (egress-subnets,ingress-address,private-address) whose values are plain strings, not JSON, so loading unit data into a dataclass or Pydantic model raisedjson.decoder.JSONDecodeError: Extra data— even when the class doesn't declare those fields.This change filters by the class fields first and only decodes matching values, so keys that are not part of the data class are ignored without ever reaching the decoder. Round-trip semantics are unchanged: declared fields are still decoded with
json.loads(or the customdecoder), so types survivesave()/load()round trips.Out of scope: for classes that are neither dataclasses nor Pydantic models,
_juju_fields()cannot determine the fields, so the whole databag is still decoded and passed to__init__(documented behaviour — such classes were already incompatible with unit databags via the unexpected keyword arguments).Testing
test_relation_load_unit_data_ignores_juju_keys, parametrized over dataclass, Pydantic dataclass andBaseModel: saves to and loads from the unit databag with the Juju-populated keys present (ops-scenario's defaults). It fails withJSONDecodeError: Extra databefore the fix and passes after.save/loadin_on_start) via ops-scenario: same traceback as reported before the fix, loads correctly after.test_model_relation_data_class.py(66) andtest_model.py(257) all pass;tox -e lintclean.Fixes #2591