feat(snap): add snap aliases module#571
Conversation
| if typing.TYPE_CHECKING: | ||
| from collections.abc import Mapping | ||
|
|
||
| _SNAP = 'lxd' |
There was a problem hiding this comment.
I know there's an issue for having dedicated test snaps, but in the meantime, isn't lxd rather large to be using?
|
|
||
| def _alias_exists() -> bool: | ||
| info = _list_aliases().get(_SNAP, {}).get(_ALIAS) | ||
| return info is not None and info.get('command') == f'{_SNAP}.{_APP}' |
There was a problem hiding this comment.
I guess strictly speaking this is the definition of it existing and working. I wonder if a functional test ought to check the higher level "I can run the command" rather than only "snap db has the value". But maybe that falls out of testing "snap library" and into testing snapd.
| ensure_installed(_SNAP) | ||
| _cleanup_alias() | ||
| _snapd_aliases.alias(_SNAP, _APP, _ALIAS) | ||
| _snapd_aliases.alias(_SNAP, _APP, _ALIAS) # Second call — no error. |
There was a problem hiding this comment.
To really test idempotency this should check it exists after both calls. Otherwise it maybe just worked the second time and failed the first.
| ensure_installed(_SNAP) | ||
| _cleanup_alias() | ||
| _snapd_aliases.alias(_SNAP, _APP, _ALIAS) | ||
| _snapd_aliases.alias(_SNAP, 'lxd', _ALIAS) # Different app, same snap — no error. |
There was a problem hiding this comment.
I think this should be testing that the alias exists (both times) not just that there is no error.
| alias_name: The alias to remove. | ||
|
|
||
| Raises: | ||
| ChangeError: if the unalias change fails after starting. |
There was a problem hiding this comment.
Technically correct, but maybe more friendly as
| ChangeError: if the unalias change fails after starting. | |
| ChangeError: if the alias removal fails after starting. |
|
|
||
| Raises: | ||
| ChangeError: if the unalias change fails after starting. | ||
| APIError: if the alias does not exist (e.g. was never created, or the snap it |
There was a problem hiding this comment.
| APIError: if the alias does not exist (e.g. was never created, or the snap it | |
| APIError: if the alias does not exist (for example, was never created, or the snap it |
| logger = logging.getLogger(__name__) | ||
|
|
||
|
|
||
| def alias(snap: str, app: str, alias_name: str) -> None: |
There was a problem hiding this comment.
What about dropping "alias"? It's there in the function name and pretty implied.
| def alias(snap: str, app: str, alias_name: str) -> None: | |
| def alias(snap: str, app: str, name: str) -> None: |
Or, simplifying in a different way, but maybe more explicit even though it keeps the duplication:
| def alias(snap: str, app: str, alias_name: str) -> None: | |
| def alias(snap: str, app: str, alias: str) -> None: |
(Maybe linters don't like the shadow there, but it's not like this is recursive or ever would be.)
2 participants
This PR adds the snap library functionality using the
/v2/aliasesendpoint, mirroring the snap CLI commandssnap aliasandsnap unalias.