Code review comment for lp:~blake-rouse/maas/commissioning-get-disk-info

Revision history for this message
Raphaƫl Badin (rvb) wrote :

> Currently if this script fails then all of commissioning fails, which would allow the user to
> view the output where the exception would be outputted. The only way I can think around this is
> to remove disks as commands for that disk fail, which might be even more confusing for the user.

I'm a bit concerned about releasing code that might prevent the nodes from being commissioned at all if the parsing fails (given that the commands we run and the parsing we do are non-trivial). Now, I'm fine with releasing this as is as long as we preform extensive QA on this and test it on all the machines that we have: the lab, the garage MAAS, Jason's lab, OIL, etc. Can you please do this?

fwiw, I really think this ugly code could be best done with a regular expression but I'm not going to block this branch for it :)

+ info_line = info_line.strip()
+ if info_line == "":
+ continue
+ _, info = info_line.split(" ", 1)
+ if "=" not in info:
+ continue
+ k, v = info.split("=", 1)

review: Approve

« Back to merge proposal