Code review comment for lp:~fwereade/pyjuju/env-constraints

Sorry, didn't see the comments on patch set 1.

https://codereview.appspot.com/5957043/diff/1/juju/control/constraints_get.py
File juju/control/constraints_get.py (right):

https://codereview.appspot.com/5957043/diff/1/juju/control/constraints_get.py#newcode51
juju/control/constraints_get.py:51: yield
esm.set_config_state(env_config, environment.name)
On 2012/03/29 19:52:53, hazmat wrote:
> so why are we syncing env on get-constraints.. the constraints are
already
> actualized on the object being inspected. if its a legacy environment
we should
> just log warning and exit.

Addressed in later comments & branches.

https://codereview.appspot.com/5957043/diff/1/juju/control/constraints_set.py
File juju/control/constraints_set.py (right):

https://codereview.appspot.com/5957043/diff/1/juju/control/constraints_set.py#newcode90
juju/control/constraints_set.py:90: yield
esm.set_config_state(env_config, environment.name)
On 2012/03/29 19:52:53, hazmat wrote:
> Some comments on the reasoning for this here and in get-constraints
would be
> good.

Addressed in later comments and branches

https://codereview.appspot.com/5957043/diff/1/juju/control/tests/test_bootstrap.py
File juju/control/tests/test_bootstrap.py (right):

https://codereview.appspot.com/5957043/diff/1/juju/control/tests/test_bootstrap.py#newcode7
juju/control/tests/test_bootstrap.py:7: from juju.lib.mocker import ANY
On 2012/03/29 19:52:53, hazmat wrote:
> not needed.

Done.

https://codereview.appspot.com/5957043/diff/1/juju/control/tests/test_initialize.py
File juju/control/tests/test_initialize.py (right):

https://codereview.appspot.com/5957043/diff/1/juju/control/tests/test_initialize.py#newcode41
juju/control/tests/test_initialize.py:41: def test_bad_data(self):
On 2012/03/29 19:52:53, hazmat wrote:
> general comment, its really nice to have doc strings on tests for
other devs,
> even if its just a single line to express intent, and assuming the
test name
> itself isn't obvious. ie test_bad_constraints_data or
> this is a test that the admin initialize exits 1 on bad constraints
data.

Done.

https://codereview.appspot.com/5957043/