Use snapd REST api to change snap config #138
Conversation
| @@ -332,19 +333,17 @@ def get(self, key: Optional[str], *, typed: bool = False) -> Any: | |||
|
|
|||
| return self._snap("get", [key]).strip() | |||
|
|
|||
| def set(self, config: Dict[str, Any], *, typed: bool = False) -> str: | |||
| def set(self, config: Dict[str, Any], *, typed: bool = False) -> None: | |||
There was a problem hiding this comment.
Technically this is a breaking change, but it previously returned whatever progress garbage that snap set printed to stdout, so charmers almost certainly weren't using it. So we're okay with changing this type to None. @james-garner-canonical to double check charm usage.
There was a problem hiding this comment.
Changed to private, thanks!
There was a problem hiding this comment.
I had a look at the 400+ charms I know bout, and none of them use the return value, so this shouldn't break anyone (and it's a nice improvement)
| @@ -65,7 +66,11 @@ def test_snap_refresh(): | |||
| def test_snap_set_and_get_with_typed(): | |||
| cache = snap.SnapCache() | |||
| lxd = cache["lxd"] | |||
| lxd.ensure(snap.SnapState.Latest, channel="latest") | |||
| try: | |||
There was a problem hiding this comment.
From Weii: this is to address instability in the GitHub Runners when running these tests.
There was a problem hiding this comment.
Looks good to me, thanks @weiiwang01.
I haven't gone over the new tests with a fine-toothed comb. Added @james-garner-canonical as a second reviewer.
| @@ -332,19 +333,17 @@ def get(self, key: Optional[str], *, typed: bool = False) -> Any: | |||
|
|
|||
| return self._snap("get", [key]).strip() | |||
|
|
|||
| def set(self, config: Dict[str, Any], *, typed: bool = False) -> str: | |||
| def set(self, config: Dict[str, Any], *, typed: bool = False) -> None: | |||
There was a problem hiding this comment.
I had a look at the 400+ charms I know bout, and none of them use the return value, so this shouldn't break anyone (and it's a nice improvement)
|
|
||
| client = snap.SnapClient() | ||
| with patch.object(client, "_request_raw", _request_raw), patch.object(time, "sleep"): | ||
| client._put_snap_conf("test", {"foo": "bar"}) |
There was a problem hiding this comment.
I guess ideally we'd call the public Snap.set instead of the private SnapClient._put_snap_client method, but that's not a blocker
Use the snapd REST API to change snap configurations instead of using the
snapcommand.This improves security in case a secret in command line arguments are logged.