feat: coerce nested dataclass fields in Relation.load - #2741
Open
tonyandrewmeyer wants to merge 8 commits into
Open
feat: coerce nested dataclass fields in Relation.load#2741tonyandrewmeyer wants to merge 8 commits into
tonyandrewmeyer wants to merge 8 commits into
Conversation
Split out of canonical#2557, where this shipped alongside the ops_tracing de-pydantic work. Relation.load now recursively constructs nested dataclasses and coerces Enum field values when the target is a non-pydantic dataclass, so callers get typed nested objects instead of raw decoded dicts. This carries over the original implementation from canonical#2557 unmodified; the review at REVIEW-2557.md found several holes in it, fixed in the commits that follow. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J1dP9XqjiuoYz4oHjWYAQ6
….load _coerce_field returned early for anything whose typing.get_origin wasn't list/tuple/set/frozenset, which includes Optional[X] (a Union) and dict[str, X] - both common shapes for nested-model fields. A charm reading data.inner.a on an Optional[Nested] field got an AttributeError instead, since inner stayed a plain dict. Coerce a Union against its single non-None member (a Union of more than one concrete type has no way to pick a target, so it still passes through as-is), and a dict/Mapping's values against the value type. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J1dP9XqjiuoYz4oHjWYAQ6
…ionally origin in (list, tuple) shared one branch, so tuple[X, ...] fields came back as a list, and every element of a fixed-length tuple[X, Y] was coerced against args[0], leaving Y untouched. Split the branch: a tuple stays a tuple, a variable-length tuple[X, ...] (args[-1] is Ellipsis) coerces every element against X, and a fixed-length tuple coerces each position against its own type. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J1dP9XqjiuoYz4oHjWYAQ6
The guard used __is_pydantic_dataclass__, which only exists from pydantic 2.11. Measured across installed versions, it's absent on 2.0.3, 2.4.2, 2.6.4 and 2.10.6. ops declares no pydantic dependency - the charm's own pin decides - and ops's test extra permits 2.10.x, so a charm pinned there got ops's pre-coercion applied ahead of pydantic's own validators, and any extra kwargs pydantic would have accepted silently dropped. '__pydantic_validator__' in cls.__dict__ is what pydantic.dataclasses.is_pydantic_dataclass itself checks for, and was present on every version tested from 2.0.3 up. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J1dP9XqjiuoYz4oHjWYAQ6
… be resolved get_type_hints(cls) resolves every field's annotation eagerly, so a TYPE_CHECKING-only import with no runtime name raised NameError even for relation data that never touched the affected field - a regression against main's cls(**data) path, which didn't need the hints at all. ops's own ruff config disables TC001/2/3, so charms following ops's own conventions are the ones most likely to hit this. Fall back to the un-coerced cls(**data) when hints can't be resolved. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J1dP9XqjiuoYz4oHjWYAQ6
…ional args The `not args` guard meant any positional argument silently turned off coercion entirely: relation.load(MyData, event.app) coerced, and relation.load(MyData, event.app, something) did not, with nothing telling the caller so. args are matched to cls's leading dataclass fields by position, so there is nothing to coerce them against - but the remaining fields, still supplied from the relation data, can and should keep being coerced. _build_dataclass now takes the positional args through and only skips coercion for the fields they fill. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J1dP9XqjiuoYz4oHjWYAQ6
The docstring still said the data "is passed to the data class's __init__ method as keyword arguments", which after recursive coercion holds only for pydantic targets and flat dataclasses. State which field types are coerced, which pass through unchanged, and what happens when positional arguments or unresolvable type hints are involved. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J1dP9XqjiuoYz4oHjWYAQ6
tonyandrewmeyer
commented
Sep 10, 2026
tonyandrewmeyer
marked this pull request as ready for review
September 10, 2026 04:53
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
2 participants
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.
Relation.loadpasses the decoded relation data straight to the data class's__init__, so a field whose own type is a dataclass or an enum arrives as a plaindictorstr, and the attribute access the charm does next fails. This PR changes that to coerce each field against its type hint before construction, recursively, for plain dataclasses only - Pydantic models and pydantic dataclasses keep doing their own coercion and validation.list,setandfrozensetcoerce their elements; a fixed-length tuple coerces each position against its own type and stays a tuple;dict/Mappingcoerce their values.Optional/Unionfield is coerced against its single non-Nonemember. A union of more than one concrete type is passed through as-is, since there's no way to tell which member to coerce against.Split out of #2557, which is where this came from and which needs it:
ops_tracing's de-vendored databag models have nested dataclasses one level deep, and its suite doesn't pass without this. It seems like it's valuable outside of that (if we end up going with using the charmlibs libraries, for example), and reviewing it separately makes it easier to review #2557 focussing specifically on what it's about.