Code review comment for ~tiago.pasqualini/layer-snap:snap_list

Revision history for this message
Tom Haddon (mthaddon) wrote :

One comment inline about docstring.

The way this is set up currently we're going to have a different exception from previous. With the previous code we'd raise an IndexError if the snap existed but wasn't installed, or a subprocess.CalledProcessError if the snap didn't exist. This code will now raise a KeyError in both cases.

I don't think that's terrible, but I think it might be nice to raise a custom exception so that charm writers can have some stability there by inspecting that.

« Back to merge proposal