Skip to content

fix: a collection of small model fixes - #2852

Open
tonyandrewmeyer wants to merge 8 commits into
canonical:mainfrom
tonyandrewmeyer:fix/model-small-fixes
Open

tonyandrewmeyer wants to merge 8 commits into
canonical:mainfrom
tonyandrewmeyer:fix/model-small-fixes

Conversation

@tonyandrewmeyer

@tonyandrewmeyer tonyandrewmeyer commented Oct 25, 2026 •

Copy link
Copy Markdown
Collaborator

A collection of small model.py fixes, none of which seemed worth a PR of its own:

  • A charm subclassing a built-in status (to add a field, say) replaced the built-in in StatusBase._statuses, so from_name() constructed the subclass when reading the status back from Juju and raised TypeError on every read. Built-in names now keep their built-in class.
  • RelationData held only a weakref.proxy to its Relation (since the original 2019 code, with no recorded reason). A Relation that the model doesn't cache (one fetched by an ID that relation-ids doesn't list, as happens during relation-broken) was collected as soon as the caller dropped it, and then every access through .data raised ReferenceError.
  • Secret kept a reference to the caller's content dict rather than a copy.
  • Network parsing used strict ip_network(), which raises ValueError for a CIDR with host bits set, such as the interface's own address with the subnet's prefix length.
  • LazyNotice and LazyCheckInfo recursed forever in __getattr__ when copied, because copy looks the private attributes up before __init__ has run.
  • set_ports compared the protocol case-sensitively against opened-ports, so an upper-case protocol closed and reopened the port. open_port and close_port already lowercase it, so this brings the three into line (the type hints forbid upper case anyway).
  • An unknown secret rotation policy from secret-info-get raised ValueError.
  • The get_content docstring claimed that set_content invalidates the cached content, which the code deliberately doesn't do (and the Scenario tests assert that it doesn't). Juju's hook context returns a pending secret-set value for peek or refresh in the same hook and the tracked revision otherwise, so the docstring now says that.

tonyandrewmeyer and others added 2 commits October 25, 2026 15:57
* A charm subclassing a built-in status (to add a field, say) replaced the
  built-in in StatusBase's registry, so from_name() constructed the subclass
  when reading the status back from Juju and raised TypeError on every read.
  Built-in names now keep their built-in class.
* RelationData held only a weakref proxy to its Relation. A Relation that the
  model does not cache (fetched by an ID that relation-ids does not list, as
  during relation-broken) was collected as soon as the caller dropped it, and
  then every access through .data raised ReferenceError.
* Secret kept a reference to the caller's content dict rather than a copy.
* Network parsing used strict ip_network(), which raises ValueError for a CIDR
  with host bits set, such as the interface's own address with the subnet's
  prefix length.
* LazyNotice and LazyCheckInfo recursed forever in __getattr__ when copied,
  because copy looks the private attributes up before __init__ has run.
* set_ports compared the protocol case-sensitively against opened-ports, so an
  upper-case protocol closed and reopened the port.
* An unknown secret rotation policy from secret-info-get raised ValueError.
* The get_content docstring claimed that set_content invalidates the cached
  content, which the code deliberately does not do.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ents

Juju returns a pending secret-set value for peek or refresh in the same hook,
and the tracked revision otherwise, so say that rather than "a later hook".

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
tonyandrewmeyer and others added 2 commits November 4, 2026 19:57
The guard was in __init_subclass__, so the deprecated but public register()
could still replace a built-in status. Moving the check to _register covers
both paths.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… docstrings

With several secrets rotating, the warning gave no way to tell which one has
the policy that ops doesn't know.

The get_secret docstring still said that set_content invalidates the cached
content, which get_content's docstring no longer claims. The egress_subnets
docstring now says that host bits are discarded, because strict=False does so
silently.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@tonyandrewmeyer
tonyandrewmeyer marked this pull request as ready for review November 8, 2026 23:04
Comment thread ops/model.py Resolved
Comment thread ops/model.py Resolved
Comment thread ops/model.py Resolved
Comment thread ops/model.py Resolved
Comment thread test/test_model.py Resolved
tonyandrewmeyer and others added 4 commits November 13, 2026 12:46
Co-authored-by: Dave Wilding <tech@dpw.me>
Co-authored-by: Dave Wilding <tech@dpw.me>
Co-authored-by: Dave Wilding <tech@dpw.me>
SecretInfo.from_dict silently dropped a rotation policy that ops doesn't
know, while the backend's secret_info_get logs a warning naming the
secret. Use the same warning, and only try to parse the policy when one
is present, so that secrets without a policy don't log anything.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants