-
Notifications
You must be signed in to change notification settings - Fork 122
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
docs: clarify where StoredState is stored, and the upgrade behaviour #1416
Changes from 3 commits
1d17b1e
befaecb
11dc96f
b040f35
995eb22
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1132,14 +1132,9 @@ def set_default(self, **kwargs: Any): | |
class StoredState: | ||
"""A class used to store data the charm needs, persisted across invocations. | ||
|
||
Example:: | ||
|
||
class MyClass(ops.Object): | ||
_stored = ops.StoredState() | ||
|
||
Instances of ``MyClass`` can transparently save state between invocations by | ||
setting attributes on ``_stored``. Initial state should be set with | ||
``set_default`` on the bound object, that is:: | ||
``set_default`` on the bound object; for example:: | ||
|
||
class MyClass(ops.Object): | ||
_stored = ops.StoredState() | ||
|
@@ -1152,6 +1147,16 @@ def __init__(self, parent, key): | |
def _on_seen(self, event): | ||
self._stored.seen.add(event.uuid) | ||
|
||
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`. | ||
""" | ||
|
||
def __init__(self): | ||
|
@@ -1245,7 +1250,12 @@ def _wrapped_repr(obj: '_StoredObject') -> str: | |
|
||
|
||
class StoredDict(typing.MutableMapping[Hashable, Any]): | ||
"""A dict-like object that uses the StoredState as backend.""" | ||
"""A dict-like object that uses the StoredState as backend. | ||
|
||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How does Ops know whether to create a StoredDict, StoredList, or StoredSet when you create/access a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When you set the attribute it gets wrapped and similarly unwrapped when accessed. It does an |
||
""" | ||
|
||
def __init__(self, stored_data: StoredStateData, under: Dict[Hashable, Any]): | ||
self._stored_data = stored_data | ||
|
@@ -1280,7 +1290,12 @@ def __eq__(self, other: Any): | |
|
||
|
||
class StoredList(typing.MutableSequence[Any]): | ||
"""A list-like object that uses the StoredState as backend.""" | ||
"""A list-like object that uses the StoredState as backend. | ||
|
||
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 list-like data. | ||
""" | ||
|
||
def __init__(self, stored_data: StoredStateData, under: List[Any]): | ||
self._stored_data = stored_data | ||
|
@@ -1354,7 +1369,12 @@ def __ge__(self, other: Any): | |
|
||
|
||
class StoredSet(typing.MutableSet[Any]): | ||
"""A set-like object that uses the StoredState as backend.""" | ||
"""A set-like object that uses the StoredState as backend. | ||
|
||
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 set-like data. | ||
""" | ||
|
||
def __init__(self, stored_data: StoredStateData, under: Set[Any]): | ||
self._stored_data = stored_data | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
toTrue
when running :meth:ops.main
(but this is not recommended)."There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
podspec charms are deprecated, aren't they? Second guessing myself ..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes :). You have to find that info in the Juju docs, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense!