robur-coop / albatross

Albatross: orchestrate and manage MirageOS unikernels with Solo5
ISC License
142 stars 17 forks source link

Ergonomics #41

Closed reynir closed 3 years ago

reynir commented 3 years ago

The commands block, info, and policy currently return an error in the case that there are no block devices, unikernels or policies respectively. At the client this results in a dangerous-looking [WARNING] when those commands are run on a fresh install.

This change makes the daemon return success with an empty list instead of an error. Instead, pretty printing is modified to make a note of the list being empty.

This PR contains two commits. The previous would make the daemon return a string with the message. I think it's cleaner if it's up to the client to handle empty lists. I can squash the commits into one if preferred.

hannesm commented 3 years ago

Thanks for your PR. Since the Error vs Ok leads to different exit codes of the client, I'm not entirely sure about the change in semantics. Maybe the commands are overloaded? Using Block_info <id> as example, a client may want to:

But considering that Block_add errors if the given block device already exists, and the above (execute block_info just before block_add) is racy, I agree with the proposed changes here (see minor nits inline). The remaining command, Unikernel_get , should be adapted to the same behaviour. EDIT: no, the behaviour of Unikernel_get is fine as is, since it requires a single unikernel name (and does not work in a "retrieve all unikernels" way - in contrast to block_info, unikernel_info, policy_info).

EDIT: I rebased your branch and pushed a commit which simplifies the code. Thanks for your suggestion, good to be squashed and merged when CI is green.