Merge lp:~hazmat/pyjuju/env-id into lp:pyjuju

[Kapil Thangavelu]

Environments have uuids (regression fix for subs)

Previous environment uuid work caused issues with subordinates, added
an additional test and one line fix for. Fixes bugs

https://bugs.launchpad.net/juju/+bug/1100245
https://bugs.launchpad.net/juju/+bug/1100348

https://codereview.appspot.com/7092055/

[Kapil Thangavelu (hazmat)]

Reviewers: mp+143188_code.launchpad.net,

Message:
Please take a look.

Description:
Environments have uuids.

Data collection tools that want to operate on multiple environments need
to be unambigiously refererence an identity for the environment. This
branch introduces a JUJU_ENV_ID variable, available in hooks for this
purpose.

https://code.launchpad.net/~hazmat/juju/env-id/+merge/143188

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/7092055/

Affected files:
   A [revision details]
   M juju/agents/tests/test_unit.py
   M juju/hooks/invoker.py
   M juju/hooks/tests/test_executor.py
   M juju/hooks/tests/test_invoker.py
   M juju/machine/tests/data/test_get_container
   M juju/machine/tests/test_unit_deployment.py
   M juju/machine/unit.py
   M juju/providers/common/cloudinit.py
   M juju/state/environment.py
   M juju/state/initialize.py
   M juju/state/tests/test_environment.py
   M juju/state/tests/test_initialize.py
   M juju/unit/deploy.py
   M juju/unit/tests/test_address.py
   M juju/unit/tests/test_deploy.py
   M juju/unit/tests/test_lifecycle.py

[Benjamin Saller (bcsaller)]

LGTM. It will be worth adding a note to the hook docs that this env var
is present but that can happen later if you want. Given that this will
have to be ported to juju-core making a note about its availability is
nice.

Also I suspect there will be changes in rapi branches to include the env
uuid either via a new call or as part of the connect message header.

https://codereview.appspot.com/7092055/

[Kapil Thangavelu (hazmat)]

On 2013/01/15 14:42:47, bcsaller wrote:
> LGTM. It will be worth adding a note to the hook docs that this env
var is
> present but that can happen later if you want. Given that this will
have to be
> ported to juju-core making a note about its availability is nice.

> Also I suspect there will be changes in rapi branches to include the
env uuid
> either via a new call or as part of the connect message header.

thanks for the review. docs and rapi are in different branches so need
to be tackled separately. i fired off an email to juju-dev re the
functionality.

https://codereview.appspot.com/7092055/

[Kapil Thangavelu (hazmat)]

On Tue, Jan 15, 2013 at 9:34 AM, Kapil Thangavelu <
<email address hidden>> wrote:

> On 2013/01/15 14:42:47, bcsaller wrote:
> > LGTM. It will be worth adding a note to the hook docs that this env
> var is
> > present but that can happen later if you want. Given that this will
> have to be
> > ported to juju-core making a note about its availability is nice.
>
> > Also I suspect there will be changes in rapi branches to include the
> env uuid
> > either via a new call or as part of the connect message header.
>
>
> thanks for the review. docs and rapi are in different branches so need
> to be tackled separately. i fired off an email to juju-dev re the
> functionality.
>
>
per core comments switching this to JUJU_ENV_UUID.

cheers,

Kapil

[Kapil Thangavelu (hazmat)]

*** Submitted:

Environments have uuids.

Data collection tools that want to operate on multiple environments need
to be unambigiously refererence an identity for the environment. This
branch introduces a JUJU_ENV_UUID variable, available in hooks for this
purpose.

R=bcsaller
CC=
https://codereview.appspot.com/7092055

https://codereview.appspot.com/7092055/

[Kapil Thangavelu (hazmat)]

Please take a look.

https://codereview.appspot.com/7092055/

[Jim Baker (jimbaker)]

On 2013/01/16 19:02:23, hazmat wrote:
> Please take a look.

+1, LGTM

https://codereview.appspot.com/7092055/

[Kapil Thangavelu (hazmat)]

*** Submitted:

Environments have uuids (regression fix for subs)

Previous environment uuid work caused issues with subordinates, added
an additional test and one line fix for. Fixes bugs

https://bugs.launchpad.net/juju/+bug/1100245
https://bugs.launchpad.net/juju/+bug/1100348

R=jimbaker
CC=
https://codereview.appspot.com/7092055

https://codereview.appspot.com/7092055/

=== modified file 'juju/unit/deploy.py'
--- juju/unit/deploy.py	2013-01-14 21:49:10 +0000
+++ juju/unit/deploy.py	2013-01-16 19:03:27 +0000
@@ -39,8 +39,8 @@
     def start(self, provider_type=None):
         """Starts the unit deployer."""
         # Find out what provided the machine, and how to deploy units.
+        settings = GlobalSettingsStateManager(self.client)
         if provider_type is None:
-            settings = GlobalSettingsStateManager(self.client)
             provider_type = yield settings.get_provider_type()
 
         self.deploy_factory = get_deploy_factory(provider_type)

=== modified file 'juju/unit/tests/test_deploy.py'
--- juju/unit/tests/test_deploy.py	2013-01-14 21:49:10 +0000
+++ juju/unit/tests/test_deploy.py	2013-01-16 19:03:27 +0000
@@ -66,6 +66,14 @@
                          UnitMachineDeployment)
 
     @inlineCallbacks
+    def test_start_with_provider_type(self):
+        # bug test against 1100245
+        self.unit_manager = UnitDeployer(
+            self.client, str(self.machine_state.id), self.juju_dir)
+        yield self.unit_manager.start("subordinate")
+        self.assertEqual(self.unit_manager.env_id, "snowflake")
+
+    @inlineCallbacks
     def test_charm_download(self):
         """Downloading a charm should store the charm locally."""
         yield self.unit_manager.download_charm(self.charm_state)