Conversation
| """ | ||
| return is_installed() | ||
|
|
||
| def list_instances(self) -> Collection[LXDInstance]: |
There was a problem hiding this comment.
suggestion: Add a test, at least to make sure we detect that this function should be updated if/when the LXDInstance class is updated? (same for the multipass provider)
There was a problem hiding this comment.
Agreed, a simple unit test for both would be useful.
|
what's left to do on this draft before merging? |
| install() | ||
| ensure_multipass_is_ready() | ||
|
|
||
| def list_instances(self) -> Collection[MultipassInstance]: |
There was a problem hiding this comment.
Does this list all vms? Not just vms for the current application?
The API feels ambiguous or underdocumented, if the lxd provider returns instances for the current application (via project) and the multipass provider returns all vms.
There was a problem hiding this comment.
In my opinion, the call for multipass should only return VMs starting with <app-name>- (for example, snapcraft- or rockcraft-).
This approach is naive, because if someone creates their own multipass vm like snapcraft-dev, we would unintentionally delete that. A follow-up PR could add some metadata or fingerprint to the VMs created by craft-providers, so we only delete our VMs.
| """ | ||
| return is_installed() | ||
|
|
||
| def list_instances(self) -> Collection[LXDInstance]: |
There was a problem hiding this comment.
Agreed, a simple unit test for both would be useful.
|
Hi @me6262, I think the remaining work is:
|
|
I can work on this. |
@Aeonoi, great! You can create a new PR and either use this as a starting point, or start from scratch. |
5 participants
tox?Fixes #639
Requires #644