-
Notifications
You must be signed in to change notification settings - Fork 28
feat(snap): add snap interfaces module #570
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
base: main
Are you sure you want to change the base?
Changes from all commits
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 |
|---|---|---|
| @@ -0,0 +1,108 @@ | ||
| # Copyright 2026 Canonical Ltd. | ||
| # | ||
| # Licensed under the Apache License, Version 2.0 (the "License"); | ||
| # you may not use this file except in compliance with the License. | ||
| # You may obtain a copy of the License at | ||
| # | ||
| # http://www.apache.org/licenses/LICENSE-2.0 | ||
| # | ||
| # Unless required by applicable law or agreed to in writing, software | ||
| # distributed under the License is distributed on an "AS IS" BASIS, | ||
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
|
|
||
| """Snap interface operations, implemented as calls to the snapd API's /v2/interfaces endpoint.""" | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| from typing import Any | ||
|
|
||
| from . import _client, _errors | ||
|
|
||
| # /v2/interfaces | ||
|
|
||
|
|
||
| def connect( | ||
| plug_snap: str, plug: str, slot_snap: str | None = None, slot: str | None = None, / | ||
| ) -> None: | ||
| """Connect a snap and plug, to a target snap and slot. | ||
|
|
||
| Connecting an already-connected plug and slot succeeds silently. | ||
|
|
||
| Args: | ||
| plug_snap: The name of the snap providing the plug. | ||
| plug: The name of the plug on ``plug_snap``. | ||
| slot_snap: The name of the snap providing the slot. If omitted, snapd auto-resolves | ||
| the slot, typically to the system snap (``snapd`` or ``core``). | ||
| slot: The name of the slot on ``slot_snap``. If omitted, snapd auto-resolves it. | ||
|
|
||
| Raises: | ||
| APIError: if the plug snap or slot snap is not installed, or the named plug or slot | ||
| does not exist. The error has an empty ``kind``; inspect ``message`` for details. | ||
| ChangeError: if the connection fails after starting (for example, an interface hook | ||
| errors). | ||
| """ | ||
| data = { | ||
| 'action': 'connect', | ||
| 'plugs': [{'snap': plug_snap, 'plug': plug}], | ||
| 'slots': [{'snap': slot_snap or '', 'slot': slot or ''}], | ||
| } | ||
| _client.post('/v2/interfaces', body=data) | ||
|
|
||
|
|
||
| def disconnect( | ||
| plug_or_slot_snap: str, | ||
| plug_or_slot: str, | ||
| slot_snap: str | None = None, | ||
| slot: str | None = None, | ||
| /, | ||
| *, | ||
| forget: bool = False, | ||
| ) -> None: | ||
| """Disconnect a plug from a slot. | ||
|
|
||
| May be called in two forms: | ||
|
Contributor
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. I really don't like this as a Python API style. Did this get discussed and resolved already when everything was in one PR? kwonly would solve this, or two methods would solve this. I definitely need convincing that positional args can be entirely different objects based on the number provided. |
||
|
|
||
| - ``(snap, plug_or_slot)`` disconnects everything connected to the named plug or slot | ||
| on ``snap``. | ||
| - ``(plug_snap, plug, slot_snap[, slot])`` disconnects the plug from the slot. ``slot`` | ||
| may be omitted to disconnect the plug from any slot on ``slot_snap``. | ||
|
|
||
| Disconnecting a plug and slot that are not connected is a no-op and does not raise | ||
| (the underlying ``interfaces-unchanged`` error is suppressed, mirroring the snap CLI). | ||
|
|
||
| Args: | ||
| plug_or_slot_snap: The snap providing the plug (explicit form) or the snap providing | ||
| the plug or slot to disconnect (two-argument form). | ||
| plug_or_slot: The plug on ``plug_or_slot_snap`` (explicit form) or the plug or slot | ||
| name to disconnect (two-argument form). | ||
| slot_snap: The snap providing the slot. Omit for the two-argument form. | ||
| slot: The slot on ``slot_snap``. May be omitted to match any slot on ``slot_snap``. | ||
| forget: If ``True``, also forget any manual connection preference, so the interface | ||
|
Contributor
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. I'm not super familiar with snap interfaces. I assume people who are would understand this, but maybe we should help others out more? Is there a way to use the library to set a manual connection preference? Where would I go to learn more about what this means, as a library user? Also, I think we would generally use |
||
| is not automatically reconnected on the next refresh. | ||
|
|
||
| Raises: | ||
| APIError: if a named snap is not installed, or the named plug or slot does not exist. | ||
| The error has an empty ``kind``; inspect ``message`` for details. | ||
| ChangeError: if the disconnection fails after starting (for example, an interface hook | ||
| errors). | ||
| """ | ||
| data: dict[str, Any] = {'action': 'disconnect'} | ||
| if slot_snap is None: | ||
| assert slot is None | ||
| # Called with 2 arguments, treat as `snap disconnect <snap>:<slot>`. | ||
| data['plugs'] = [{'snap': '', 'plug': ''}] | ||
| data['slots'] = [{'snap': plug_or_slot_snap, 'slot': plug_or_slot}] | ||
| else: | ||
| # Called with 3 or 4 arguments, treat as `snap disconnect <snap>:<plug> <snap>:<slot>`. | ||
| data['plugs'] = [{'snap': plug_or_slot_snap, 'plug': plug_or_slot}] | ||
| data['slots'] = [{'snap': slot_snap, 'slot': slot or ''}] | ||
| if forget: | ||
| data['forget'] = True | ||
| # NOTE: Unlike connect, the API raises interfaces-unchanged if already disconnected. | ||
| # We suppress this to make disconnect symmetric with connect (following the snap CLI). | ||
| try: | ||
| _client.post('/v2/interfaces', body=data) | ||
| except _errors._InterfacesUnchangedError: | ||
| pass # Follow the snap CLI's lead and suppress this error. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,220 @@ | ||
| #!/usr/bin/env python3 | ||
| # Copyright 2026 Canonical Ltd. | ||
| # See LICENSE file for licensing details. | ||
|
|
||
| """Functional tests for _snapd_interfaces: connect, disconnect.""" | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import dataclasses | ||
| import typing | ||
| from typing import Any | ||
|
|
||
| import pytest | ||
|
|
||
| from charmlibs.snap import _client, _errors, _snapd_interfaces | ||
| from conftest import ensure_installed | ||
|
|
||
| _SNAP = 'htop' | ||
| _PLUG = 'mount-observe' | ||
| # snapd auto-resolves the mount-observe slot to snapd. | ||
| _SLOT_SNAP = 'snapd' | ||
| _SLOT = 'mount-observe' | ||
|
|
||
| # A snap name that is never installed — used for error paths where any absent | ||
| # snap produces the same error response, avoiding unnecessary remove operations. | ||
| _ABSENT_SNAP = 'this-snap-does-not-exist-xyz-abc-123' | ||
|
|
||
|
|
||
| # --------------------------------------------------------------------------- | ||
| # Test helpers and possible future candidates for library public API. | ||
| # --------------------------------------------------------------------------- | ||
|
|
||
|
|
||
| @dataclasses.dataclass | ||
| class _Plug: | ||
| interface: str | ||
| plug: str | ||
|
|
||
|
|
||
| @dataclasses.dataclass | ||
| class _Slot: | ||
| interface: str | ||
| slot: str | ||
|
|
||
|
|
||
| def _list_plugs(snap: str, connected_only: bool = False) -> list[_Plug]: | ||
| interfaces = _list_interfaces(snap, connected_only=connected_only) | ||
| return [ | ||
| _Plug(interface=i['name'], plug=p['plug']) | ||
| for i in interfaces | ||
| for p in i.get('plugs', []) | ||
| if p['snap'] == snap | ||
| ] | ||
|
|
||
|
|
||
| def _list_slots(snap: str, connected_only: bool = False) -> list[_Slot]: | ||
| interfaces = _list_interfaces(snap, connected_only=connected_only) | ||
| return [ | ||
| _Slot(interface=i['name'], slot=s['slot']) | ||
| for i in interfaces | ||
| for s in i.get('slots', []) | ||
| if s['snap'] == snap | ||
| ] | ||
|
|
||
|
|
||
| def _list_interfaces( | ||
| snap: str | None = None, connected_only: bool = False | ||
| ) -> list[dict[str, Any]]: | ||
| """List snap interfaces.""" | ||
| query = {'select': 'connected' if connected_only else 'all', 'slots': 'true', 'plugs': 'true'} | ||
| interfaces = _client.get('/v2/interfaces', query=query) | ||
| assert isinstance(interfaces, list) | ||
| interfaces = typing.cast('list[dict[str, Any]]', interfaces) | ||
| if snap is None: | ||
| return interfaces | ||
| return [ | ||
| i | ||
| for i in interfaces | ||
| if any(p['snap'] == snap for p in i.get('plugs', [])) | ||
| or any(s['snap'] == snap for s in i.get('slots', [])) | ||
| ] | ||
|
|
||
|
|
||
| # --------------------------------------------------------------------------- | ||
|
|
||
|
|
||
| def _is_connected() -> bool: | ||
| return any(p.plug == _PLUG for p in _list_plugs(_SNAP, connected_only=True)) | ||
|
|
||
|
|
||
| def _ensure_disconnected() -> None: | ||
| try: | ||
| _snapd_interfaces.disconnect(_SNAP, _PLUG) | ||
| except Exception: # noqa: S110 | ||
| pass | ||
| # Post-condition: the plug really is no longer connected. | ||
| assert not any(p.plug == _PLUG for p in _list_plugs(_SNAP, connected_only=True)) | ||
|
|
||
|
|
||
| def _ensure_connected() -> None: | ||
| # Pre-condition: the slot side (snapd) actually provides the mount-observe slot, | ||
| # otherwise the connection could never succeed. | ||
| assert any(s.slot == _SLOT for s in _list_slots(_SLOT_SNAP)) | ||
| if not _is_connected(): | ||
| _snapd_interfaces.connect(_SNAP, _PLUG) | ||
| # Post-condition: the plug is now connected. | ||
| assert _is_connected() | ||
|
|
||
|
|
||
| # --------------------------------------------------------------------------- | ||
| # connect | ||
| # --------------------------------------------------------------------------- | ||
|
|
||
|
|
||
| def test_connect(): | ||
| ensure_installed(_SNAP) | ||
| _ensure_disconnected() | ||
| assert not _is_connected() | ||
| _snapd_interfaces.connect(_SNAP, _PLUG) | ||
| assert _is_connected() | ||
|
|
||
|
|
||
| def test_connect_already_connected_no_error(): | ||
| # Connecting an already-connected plug should not raise. | ||
| ensure_installed(_SNAP) | ||
| _ensure_connected() | ||
| assert _is_connected() | ||
| _snapd_interfaces.connect(_SNAP, _PLUG) # Should not raise. | ||
| assert _is_connected() | ||
|
|
||
|
|
||
| def test_connect_nonexistent_plug_raises(): | ||
| # Connecting a nonexistent plug raises a base Error (no kind from snapd). | ||
| ensure_installed(_SNAP) | ||
| with pytest.raises(_errors.Error) as ctx: | ||
| _snapd_interfaces.connect(_SNAP, 'nonexistent-plug') | ||
| assert not ctx.value.kind | ||
| assert 'nonexistent-plug' in ctx.value.message | ||
|
|
||
|
|
||
| def test_connect_with_explicit_slot(): | ||
| # connect() accepts an explicit slot snap and slot name. | ||
| ensure_installed(_SNAP) | ||
| _ensure_disconnected() | ||
| _snapd_interfaces.connect(_SNAP, _PLUG, _SLOT_SNAP, _SLOT) | ||
| assert _is_connected() | ||
|
|
||
|
|
||
| # --------------------------------------------------------------------------- | ||
| # disconnect | ||
| # --------------------------------------------------------------------------- | ||
|
|
||
|
|
||
| def test_disconnect(): | ||
| ensure_installed(_SNAP) | ||
| _ensure_connected() | ||
| assert _is_connected() | ||
| _snapd_interfaces.disconnect(_SNAP, _PLUG) | ||
| assert not _is_connected() | ||
|
|
||
|
|
||
| def test_disconnect_not_connected_no_error(): | ||
| # Disconnecting a plug that is not connected is a no-op (interfaces-unchanged suppressed). | ||
| # This makes disconnect symmetric with connect: both succeed silently when no change needed. | ||
| ensure_installed(_SNAP) | ||
| _ensure_disconnected() | ||
| _snapd_interfaces.disconnect(_SNAP, _PLUG) # Should not raise. | ||
| assert not _is_connected() | ||
|
|
||
|
|
||
| def test_disconnect_forget_connected_no_error(): | ||
| # disconnect forget=True on a connected interface works without error. | ||
| ensure_installed(_SNAP) | ||
| _ensure_connected() | ||
| _snapd_interfaces.disconnect(_SNAP, _PLUG, forget=True) # Should not raise. | ||
|
|
||
|
|
||
| def test_disconnect_forget_not_connected_no_error(): | ||
| # disconnect forget=True on a not-connected interface is a no-op | ||
| # (interfaces-unchanged suppressed, same as without forget=True). | ||
| ensure_installed(_SNAP) | ||
| _ensure_disconnected() | ||
| _snapd_interfaces.disconnect(_SNAP, _PLUG, forget=True) # Should not raise. | ||
|
|
||
|
|
||
| def test_disconnect_nonexistent_plug_or_slot_raises(): | ||
| # disconnect: plug/slot name doesn't exist on the installed snap. | ||
| ensure_installed(_SNAP) | ||
| with pytest.raises(_errors.APIError) as ctx: | ||
| _snapd_interfaces.disconnect(_SNAP, 'nonexistent-slot') | ||
| assert not ctx.value.kind | ||
| assert 'no plug or slot named' in ctx.value.message | ||
|
|
||
|
|
||
| # --------------------------------------------------------------------------- | ||
| # not-installed snap (uses a never-installed name to avoid churn) | ||
| # --------------------------------------------------------------------------- | ||
|
|
||
|
|
||
| def test_connect_not_installed_snap_raises(): | ||
| with pytest.raises(_errors.APIError) as ctx: | ||
| _snapd_interfaces.connect(_ABSENT_SNAP, 'home') | ||
| assert not ctx.value.kind | ||
| assert 'not installed' in ctx.value.message | ||
|
|
||
|
|
||
| def test_disconnect_not_installed_snap_raises(): | ||
| with pytest.raises(_errors.APIError) as ctx: | ||
| _snapd_interfaces.disconnect(_ABSENT_SNAP, 'home') | ||
| assert not ctx.value.kind | ||
| assert 'not installed' in ctx.value.message | ||
|
|
||
|
|
||
| def test_connect_slot_snap_not_installed_raises(): | ||
| # connect: slot snap not installed raises APIError with empty kind. | ||
| ensure_installed(_SNAP) | ||
| with pytest.raises(_errors.APIError) as ctx: | ||
| _snapd_interfaces.connect(_SNAP, _PLUG, _ABSENT_SNAP, _SLOT) | ||
| assert not ctx.value.kind | ||
| assert 'not installed' in ctx.value.message |
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.
Why positional only? I think I would prefer keywords here, because it's awkwardly a pair of strs and then a one-or-two str 'pair'. Plug->slot works ok for people who normally read left-to-right, I think, but then you're relying on CLI familiarity or seeing the signature to know the order of the rest.
I feel like I would have gone with
Although with no default for plug, that is a little odd.
I think I'm ok with no kwonly, but I don't like positional only unless there's a motivation I'm missing.
Uh oh!
There was an error while loading. Please reload this page.
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.
Connect and disconnect both have this basic problem. The CLI uses colon-separated strings for the snap and its plug/slot, like:
Translating this to Python, I thought that separate positional arguments made the most sense. But there are other options. We've resisted creating a reified
Snaptype elsewhere, so we should probably see how far we can get with that here (so noSnap('foo').plug('bar').connect(Snap('baz').slot('bartholemew')).Sticking with the flat, functional design, a few ideas spring to mind:
tuple[str, str].I do suspect that reducing to two arguments would be a net win.
For
connect, the slot snap can be omitted entirely, or provided as a bare snap name (no slot specified).disconnectsupports a single snap form, but with different semantics: if a single colon-separated snap name is provided, whether it's a slot or plug is implicit. Actually, I'm now wondering if my implementation even gets this right -- in the single snap disconnect case, I treat it as the slot snap. I'll have to look into this further.As to the arguments being positional-only, that's why -- hard to have a name that should be used as a keyword argument if the meaning of the argument changes based on the number of arguments.
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.
Yeah, I'm onboard with that, although I do feel like it would have avoided things I don't like in other PRs too.
I see why you didn't go with this, and it does feel less Pythonic and more just exposing the snap CLI in a raw way. I do think it's interesting that the API doesn't work this way.
I think this is worth considering.
connect('somesnap')(is this possible?)connect('vlc', 'audio-record')connect(('jhack', 'ssh-read'), 'snapd')(maybe a bad example since I think snapd is the default, but it's the one I know)connect('foo', ('bar', 'face'))connect(('foo', 'one'), ('bar', 'two'))(I don't love this, but probably it is most rare?)Yeah, which is, I would argue, evidence that it's not the right API.
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 wouldn't be opposed to revisiting that decision. Maybe it's too late to do it before release, but we could always build a
Snapclass on top of the lower-level functional API later.Yeah the fact that the API explicitly accepts named 'plugs' and 'slots' arguments is a point in favour of handling that differently here.
I'll explore this option further, I kind of like it too. With the two argument form, we could use
plugandslotas the names too, and keyword arguments start to look nice again.It's not very common, though we do see this in stdlib constructs most developers would be familiar with, like
rangeandslice. But I'm hopeful that the two argument approach solves things.One thing I'm not sure about is how to spell things like
[dis]connect(plug=None, slot=(mysnap, myslot)). Maybe like that?