Code review comment for ~thibf/uvtool:thibf/uvt-kvm_wait_system_booting

Revision history for this message
Thibf (thibf) wrote :

> > I think in this case we can "just" rely on the exit code of ssh.
>
> If the remote wait script fails, then we should exit immediately rather than
> retry. So I think this should be gated on the exit value being 255 only. Any
> other exit value should result in an immediate failure (eg. by passing
> CalledProcessError through). Sorry to ask for further changes - I thought this
> is what I specified above?

I didn't get this specific behavior, my bad on this. It's fixed, consequently added a new branch in error handling because we can't print "timeout occured" while it's not the case.
>
> > I added an explicit error message for the timeout. Should we print the
> exception thrown by the sub-command ? Might help further debugging but reduce
> readability.
>
> This looks good, thanks. I don't think it's worth going into that right now.
> I'll add one minor comment inline so that this might be more easily enhanced
> later.

Applied

« Back to merge proposal