Code review comment for lp:~hazmat/pyjuju/maas-with-tags

Fix LGTM. The one thing I'm not clear on is if there's still any route
for juju to convert to a set, and stash the constraint somewhere, losing
the original string. As far as I can see not, as there's no
deserialisation function.

https://codereview.appspot.com/7365044/diff/1/juju/providers/maas/maas.py
File juju/providers/maas/maas.py (right):

https://codereview.appspot.com/7365044/diff/1/juju/providers/maas/maas.py#newcode151
juju/providers/maas/maas.py:151: params[key_to] = translate(value,
key_from, constraints)
Should we make the translate functions take (constraint, key_from)
instead? Then rather than doing the get above, it would just be:

     value = translate(constraint, key_from)
     if value is not None:
         params[key_to] = value

Not sure if that's a simplification overall, but would really like to
lose some of the complexity here.

...and thinking about it, makes the translation functions deal with the
hasattr case. Can instead this check for the key without doing the
get/conversion step?

https://codereview.appspot.com/7365044/diff/1/juju/providers/maas/tests/test_launch.py
File juju/providers/maas/tests/test_launch.py (right):

https://codereview.appspot.com/7365044/diff/1/juju/providers/maas/tests/test_launch.py#newcode124
juju/providers/maas/tests/test_launch.py:124: class
FakeWithTags(FakeMAASHTTPConnection):
I'd probably write this test with a reusable Fake class, and avoid
having closure/assertions in it, but this certainly improves our
coverage so is a good step forwards.

https://codereview.appspot.com/7365044/diff/1/juju/providers/maas/tests/test_maas.py
File juju/providers/maas/tests/test_maas.py (right):

https://codereview.appspot.com/7365044/diff/1/juju/providers/maas/tests/test_maas.py#newcode401
juju/providers/maas/tests/test_maas.py:401: constraints =
constraints.with_series('splendid')
I really liked these tests being actual unit tests that didn't need the
whole provider setup... but I think we're cursed with juju to have
everything depend on everything else.

https://codereview.appspot.com/7365044/