close
Skip to content

refactor: don't get the storage details from --help#2172

Merged
tonyandrewmeyer merged 2 commits intocanonical:mainfrom
tonyandrewmeyer:cleanup-storage-get
Nov 16, 2025
Merged

refactor: don't get the storage details from --help#2172
tonyandrewmeyer merged 2 commits intocanonical:mainfrom
tonyandrewmeyer:cleanup-storage-get

Conversation

@tonyandrewmeyer
Copy link
Copy Markdown
Collaborator

Remove the private _ModelBackend._storage_event_details method (and the _ModelBackend.STORAGE_KEY_RE only used in that method), which was only used in _main, and retrieved the storage index and location by parsing storage-get --help.

In _main, get the storage index from the JujuContext objects (so from the environment variable provided by Juju), and call _ModelBackend.storage_get directly to get the storage location.

Fixes #2109

Copy link
Copy Markdown
Contributor

@james-garner-canonical james-garner-canonical left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, nice cleanup.

Do you know how well covered this code path is in our tests?

@tonyandrewmeyer
Copy link
Copy Markdown
Collaborator Author

Do you know how well covered this code path is in our tests?

This path is exercised for any storage events, so reasonably well. Or in a more data-driven answer:

image

I don't believe the missed line would ever be reached with current Juju, but I'd rather leave it there for now.

@james-garner-canonical
Copy link
Copy Markdown
Contributor

Thanks for checking that!

Copy link
Copy Markdown
Collaborator

@benhoyt benhoyt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice to get rid of this nastiness!

@tonyandrewmeyer tonyandrewmeyer merged commit c259c87 into canonical:main Nov 16, 2025
56 checks passed
@tonyandrewmeyer tonyandrewmeyer deleted the cleanup-storage-get branch November 16, 2025 20:04
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.

Don't extract the storage key from storage-get --help

3 participants