docs: clarify where StoredState is stored, and the upgrade behaviour #1416
Conversation
ops/framework.py
Outdated
|
|
||
| Charms are not expected to use this class directly. Adding a | ||
| :class:`StoredState` attribute to a charm class will automatically use this | ||
| class to store dictionary-like data. |
There was a problem hiding this comment.
How does Ops know whether to create a StoredDict, StoredList, or StoredSet when you create/access a _stored attribute?
There was a problem hiding this comment.
When you set the attribute it gets wrapped and similarly unwrapped when accessed.
It does an isinstance check so I guess the doc should say "dictionary data" rather than
dictionary-like" even though the StoredDict class is more generic.
ops/framework.py
Outdated
| Data is stored alongside the charm (in the charm container for Kubernetes | ||
| sidecar charms, and on the machine for machine charms), except for podspec | ||
| charms and charms explicitly passing `True` for `use_juju_for_storage` when | ||
| running :meth:`ops.main` (in those cases, the data is stored in Juju). | ||
|
|
||
| For machine charms, charms are upgraded in-place on the machine, so the data | ||
| is preserved. For Kubernetes sidecar charms, when the charm is upgraded, the | ||
| pod is replaced, so any data is lost. When data should be preserved across | ||
| upgrades, Kubernetes sidecar charms should use a peer-relation for the data | ||
| instead of `StoredState`. |
There was a problem hiding this comment.
Random ideas -- I know that use_juju_for_storage isn't actually going away with 4.0, but do we want to continue to steer people away from it?
We could mention podspec charms going away in future? e.g.
"except for podspec charms (deprecated in favour of sidecare charms), where the data is stored in Juju."
And maybe move the bit about use_juju_for_storage to after the good recommendation in the last paragraph for preserving data across upgrades? e.g. as a final sentence:
"To explicitly opt into Juju data storage, set use_juju_for_storage to True when running :meth:ops.main (but this is not recommended)."
There was a problem hiding this comment.
podspec charms are deprecated, aren't they? Second guessing myself ..
There was a problem hiding this comment.
podspec charms are deprecated, aren't they? Second guessing myself ..
Yes :). You have to find that info in the Juju docs, though.
There was a problem hiding this comment.
I've adjusted it to mention podspec and use_juju_for_storage being deprecated - I agree that's worth doing.
I don't think the use_juju_for_storage mention should be at the end, though - even with a "this is not recommended" I think it promotes it too much saying that it's a way to opt in. We don't want people doing this (and they'll get a warning if they do) - it's mentioned here so people that are doing it know that it is a consequence, but if they want the storage to be in Juju then they should use a peer relation.
There was a problem hiding this comment.
Makes sense!
Adjustments to the
StoredStatedocumentation:StoredDict,StoredList, andStoredSet, note that charms would not normally create these directly and would useStoredStateinstead.As a related drive-by: remove the
jujuremovedtag foruse_juju_for_storage, since this won't actually be going away with Juju 4.0.Fixes #1270