launchpad-reviewers team mailing list archive

[Gavin Panella <gavin.panella@xxxxxxxxxxxxx>]

> Looks good, thanks for the fix. I'm building a package now from this
> branch and will run the tests once it's done.
>
> [0]
>
> 56      +    # were not repeated. See
> https://bugs.launchpad.net/maas/+bug/1131323.
> 57      +    url(r'^nodes/.*/nodes/$', nodes_handler),
>
> I /think/ that the maas-enlist bug comes from this code
> http://paste.ubuntu.com/5574676/ (extract of maas-enlist) so I think
> we can be a bit more specific than "r'^nodes/.*/nodes/" and just
> hardcode "r'^nodes/MAAS/api/1.0/nodes/".  Don't you think it would
> be better?

I don't fully understand what that code is doing; there's no
explanation what the expressions on line 5 and 8 are trying to
achieve, and I'm too jaded to try to work it out.

The expression I've added tests that "nodes/" is repeated, and that it
ends the path, which is all that's really relevant in the context of
this patterns file: "MAAS/api/1.0" is outside of its scope.

>
> [1]
>
> 27      +            })
> 28      +        self.assertEqual(httplib.OK, response.status_code)
>
> Just to be thorough, I think it's worth making sure that the node
> has been created here.

Done.

Thanks for the review :)

-- 
https://code.launchpad.net/~allenap/maas/anchor-routes/+merge/151106
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/maas/anchor-routes into lp:maas.