docs: how to design relation interfaces#372
Conversation
dimaqq
changed the title
|
@james-garner-canonical ready for review! |
| (design-relation-interfaces)= | ||
| # How to design relation interfaces | ||
|
|
||
| When designing a schema for a new interface, observe the following rules.[^op083] |
There was a problem hiding this comment.
I would say we don't need to mention the OP083 spec at all. This document should stand on its own as our recommended best practices without it. If you do want to mention it, it would be better as part of the text, with an explanation that this is a Canonical internal spec, than in this footnote format.
There was a problem hiding this comment.
Also, I wonder if it would be good to make it clearer what a schema is here: the relation data format.
| When designing a schema for a new interface, observe the following rules.[^op083] | |
| When designing the relation data format for a new interface, observe the following rules. |
| ### Library API for delta and holistic charms | ||
|
|
||
| The ideal charm library API shape is different for delta and holistic charms. Specifically, the comparison between current relation data takes place in the library for a delta charm (against old relation data), and in the charm (againts the workload) in a holistic charm. | ||
|
|
There was a problem hiding this comment.
Seems out of place in this document. We hope to add a doc next cycle to cover library API, but this doc is explicitly not about the library API itself.
| ### Library API for delta and holistic charms | |
| The ideal charm library API shape is different for delta and holistic charms. Specifically, the comparison between current relation data takes place in the library for a delta charm (against old relation data), and in the charm (againts the workload) in a holistic charm. |
| ### Process | ||
|
|
||
| First, decide what data bits should appear in the databag in the first place. | ||
|
|
||
| Then, design the JSON representaion for these bits with provisions for backwards and forwards compatibility. | ||
|
|
There was a problem hiding this comment.
I don't really understand this process, I think it's the term "data bits". Do you mean that first we decide what data needs to be transmitted over the relation (e.g. endpoints, rules), and then choose how to represent that data (e.g. integer, collection of objects)? If so, I think that could be expanded on here.
| ### Conventions | ||
|
|
||
| Using newer Pydantic, prefer the `MISSING` sentinel value over the more traditional `None`. | ||
|
|
||
| ```py | ||
| # missing field is read as <MISSING>; deleted when written out | ||
| foo: str | MISSING = MISSING | ||
|
|
||
| # missing field is read as None; written out as JSON null | ||
| foo: str | None = None | ||
| ``` | ||
|
|
There was a problem hiding this comment.
Does this recommendation actually come from OP083? I know that OP083 used MISSING as its chosen way of representing data that was left out of the databag, and I think it was a good choice for that. What I'm not convinced of is that we should be recommending that libraries adopt this experimental Pydantic feature right now -- and I think that's what we're doing here since schemas are very likely going to end up embedded in libraries at runtime.
We are saying "using newer Pydantic", so the fact that this requires at Pydantic >= 2.12 (October 2025 release), as well as Ops >= 3.6.0 (February 2026) if using Relation.save, isn't a big problem.
What does worry me is that MISSING requires import from the pydantic.experimental namespace, which means that the import location could change (or MISSING might go away entirely) in a minor Pydantic version bump. This means that libraries would need to use tight Pydantic version constraints to avoid breakage, but also libraries shouldn't use tight library constraints otherwise using multiple libraries can become painful due to dependency resolution issues.
Additionally, MISSING won't type check cleanly unless using Pyright >= 1.1.402 with enableExperimentalFeatures, and I'm not sure that we should be pushing our libraries to use enableExperimentalFeatures.
| The only changes allowed on a published interface are: | ||
|
|
||
| - adding a new field (top level or nested) | ||
| - removing a field (ideally on major version bump) |
There was a problem hiding this comment.
| - removing a field (ideally on major version bump) | |
| - removing a field: This is a backwards incompatible change, and must be clearly communicated by a major version bump of the interface library. |
Also, is there still an obligation for the library not to choke on databags that contain the dropped field, even if they don't understand it anymore?
|
|
||
| - adding a new field (top level or nested) | ||
| - removing a field (ideally on major version bump) | ||
| - with extra caution: tweaking field validators; or extending or narrowing an enum range |
There was a problem hiding this comment.
We should be more explicit about what extra caution entails.
|
|
||
| Field types cannot be narrowed, widened or changed entirely. | ||
|
|
||
| Same applies to significant changes to the range of values that a field validator accepts. |
There was a problem hiding this comment.
| Same applies to significant changes to the range of values that a field validator accepts. | |
| The same applies to significant changes to the range of values that a field validator accepts. |
This would benefit from an example of a change to the range of values that would be considered significant.
|
|
||
| Once a field has been declared, the type of that field must not be changed. | ||
|
|
||
| Field types cannot be narrowed, widened or changed entirely. |
There was a problem hiding this comment.
We should explain why, with respect to old writer / new reader and vice versa.
|
|
||
| Same applies to significant changes to the range of values that a field validator accepts. | ||
|
|
||
| Unexpected enum values should be parsed as `MISSING` or a pre-defined catch-all `UNKNOWN` value: |
There was a problem hiding this comment.
| Unexpected enum values should be parsed as `MISSING` or a pre-defined catch-all `UNKNOWN` value: | |
| Unexpected enum values should either be treated as missing (falling back to the default value or behaviour in that case) or handled with a pre-defined catch-all `UNKNOWN` value: |
| foo: Enum(A, B) | MISSING = MISSING | ||
| bar: Enum(UNKNOWN, A, B) = UNKNOWN |
There was a problem hiding this comment.
Can we make this Enum(...) type annotation valid?
|
|
||
| ### No mandatory fields | ||
|
|
||
| Top-level fields must not required or optional. |
There was a problem hiding this comment.
What's the difference between not required and optional?
| foo: str | MISSING = MISSING | ||
| ``` | ||
|
|
||
| Likewise most sub-fields must be not required or optional. |
There was a problem hiding this comment.
Why most? When does this apply and when does it not?
| session: str | MISSING = MISSING | ||
| ``` | ||
|
|
||
| A default value may be used instead in some cases. |
There was a problem hiding this comment.
Why instead? A value not being required seems to imply a default value.
Something that would be worth expanding on here is whether default values should be provided by the reader (field is missing or has a sentinel in the databag) or writer (default written to databag).
|
|
||
| ### No field reuse | ||
|
|
||
| If a field has been removed from the interface, another field with the very same name must not be added. |
There was a problem hiding this comment.
This is a pretty clear rule, but a short explanation of why introducing a new field with the same name as a removed field would be bad would be nice.
|
|
||
| ### Collections | ||
|
|
||
| Collections must be represented as arrays of objects on the wire, with few exceptions. |
There was a problem hiding this comment.
Collections must be represented as arrays of objects on the wire
It's not obvious at first glance that the important part here is "(JSON) objects". We mention at the end of this section that collections of primitives and maps are two alternatives we discourage. Let's lead with that.
with few exceptions.
What are the exceptions?
|
|
||
| Collections must be represented as arrays of objects on the wire, with few exceptions. | ||
|
|
||
| Collections must be emitted in some stable order, and the order must be ignored on reception. In other words, collections are sets. |
There was a problem hiding this comment.
We should make it clear that emission in a stable order is to avoid spurious relation changed events.
Order being ignored doesn't quite entail sets, since sets also imply uniqueness. Let's make this clear either way -- are duplicate entries forbidden? For example, perhaps I'm requesting two resources with identical specs -- if duplicates are forbidden, let's spell out that the databag format for all objects should include a unique identifier field.
| } | ||
| ``` | ||
|
|
||
| ### Secret content schema |
There was a problem hiding this comment.
How about we state this at the top of the databag schema section instead, then the reader can read all the rules knowing that they apply to shared secrets as well.
| The databag content should be structured to reflect the meaning of data, for example: | ||
|
|
||
| ```py | ||
| { | ||
| "direct": {"host": ..., "port": ...}, | ||
| "upstream": {"base_url": ..., "path": ...} | ||
| } | ||
| ``` |
There was a problem hiding this comment.
How about a "do" and a "don't" example? What we're steering authors away from here is having multiple separate keys that only make sense together, right?
|
|
||
| Data maps are strongly discouraged. An exception to this rule if when the data map key is a Juju entity with a well-known string representation, such as unit name or machine id. | ||
|
|
||
| ### URLs and URIs |
There was a problem hiding this comment.
This section feels a little out of place with the other rules. I wonder about moving it to the end. While the other rules are more general, this rule is for one specific type of value.
|
|
||
| [^op083]: OP083 - Relation Interface Design |
There was a problem hiding this comment.
See earlier comment -- let's drop the footnote.
| [^op083]: OP083 - Relation Interface Design |
|
|
||
| [Full test code](https://github.com/dimaqq/op083-samples/blob/main/test_secret_content.py) |
There was a problem hiding this comment.
It would be nice to have this code in the charmlibs repo, and since it's a single file, perhaps render it in the docs rather than jump to github, though I'd say that's optional.
Maybe it could live in the barebones example interface library's tests? Maybe a bunch of the examples from this doc could live there or in the library itself?
(CI currently enforces that the example is just a rendered version of the template, but we could remove or expand this check in favour of making the example more useful).
2 participants
Make a how-to out of the relation interface design specification (in review).
Preview: https://canonical-ubuntu-documentation-library--372.com.readthedocs.build/charmlibs/how-to/design-relation-interfaces/