LGTM, some minors on docs. 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) 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. 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) Some comments on the reasoning for this here and in get-constraints would be good. 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 not needed. 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): 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. https://codereview.appspot.com/5957043/diff/3001/juju/agents/tests/test_provision.py File juju/agents/tests/test_provision.py (right): https://codereview.appspot.com/5957043/diff/3001/juju/agents/tests/test_provision.py#newcode56 juju/agents/tests/test_provision.py:56: reactor.callLater(0.3, self.push_default_config, False) nitpick, please use the with_constraints=False invocation, its a bit easier to grok what the value is doing without hunting for the method def. https://codereview.appspot.com/5957043/diff/3001/juju/control/constraints_get.py File juju/control/constraints_get.py (right): https://codereview.appspot.com/5957043/diff/3001/juju/control/constraints_get.py#newcode57 juju/control/constraints_get.py:57: entity = yield msm.get_machine_state(name) what do you think an info log message describing the entity for which the constraints are being retrieved. ie. log.info("Fetching constraints for service %s", entity) log.info("Fetching constriants for environment") https://codereview.appspot.com/5957043/diff/3001/juju/control/constraints_set.py File juju/control/constraints_set.py (right): https://codereview.appspot.com/5957043/diff/3001/juju/control/constraints_set.py#newcode90 juju/control/constraints_set.py:90: yield esm.set_config_state(env_config, environment.name) i'd appreciate if you could document the rationale for syncing as applying to constraints, perhaps in an internals doc, referenced by the relevant code points. This will make it much easier to keep the code concerns in mind when the sync goes away. https://codereview.appspot.com/5957043/diff/3001/juju/state/environment.py File juju/state/environment.py (right): https://codereview.appspot.com/5957043/diff/3001/juju/state/environment.py#newcode49 juju/state/environment.py:49: provider = config.get_default().get_machine_provider() hmm.. this is a red flag, ie the only callers here should be privileged things (cli or provisioning agent), a doc string on the method would be good. https://codereview.appspot.com/5957043/