Code review comment for lp:~allenap/maas/ipmi-power-confusion--bug-1560830

Revision history for this message
Gavin Panella (allenap) wrote :

> This is a funny one. Some questions:
>
> On Wed, May 18, 2016 at 08:39:27AM -0000, Gavin Panella wrote:
> > + match = re.search(r'\b(on|off)\b$', stdout)
>
> I'm asking myself about the trailing \b -- is there actually a blank
> in the output, or should that actually be "\b?"?

The reason for the \b is so it doesn't match things like "karloff" or
"boron", i.e. \b doesn't match blanks, it matches word boundaries (and
consumes no characters).

Meaning the trailing \b is superfluous because the $ match for the end
of the line also implies a word boundary, hence \b(on|off)$ is, I think,
equivalent to the regex I used.

>
> Does the ipmipower output not always include a colon? If there is a
> reason not to use the colon, please add a comment.

I wrote this code given the tests already written, none of which include
hostnames or colons in the example output from ipmipower. If that is not
representative of ipmipower's output now or at any point in the past
then the tests are, well, a bit broken. If someone can categorically say
that ipmipower has only ever used the form 'hostname: status' then I can
update this code and test, as well as all the other tests.

>
> Will this fail if a hostname is simply called "on"?

No; the regex anchors to the end of the line so it won't match "on: off".

« Back to merge proposal