Merge into trunk : fix-120 : Code : juju-quickstart

Status:

Merged

Merged at revision:

88

Proposed branch:

lp://qastaging/~frankban/juju-quickstart/fix-120

Merge into:

lp://qastaging/juju-quickstart

Diff against target:

199 lines (+42/-24)

4 files modified

quickstart/__init__.py (+1/-1)
quickstart/tests/test_app.py (+1/-1)
quickstart/tests/test_watchers.py (+31/-14)
quickstart/watchers.py (+9/-8)

To merge this branch:

bzr merge lp://qastaging/~frankban/juju-quickstart/fix-120

Critical

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
Juju GUI Hackers		2014-07-04	Pending
Review via email: mp+225665@code.qastaging.launchpad.net

Description of the change

Fix quickstart for Juju 1.20.

In the addresses included in the MachineInfo,
replace NetworkScope with Scope, falling back to
NetworkScope for backwards compatibility.

Tests:
make check

QA:
Run quickstart with both 1.18 and 1.20 versions of Juju.

https://codereview.appspot.com/111860043/

Revision history for this message

Francesco Banconi (frankban) wrote on 2014-07-04:

#

Download full text (7.9 KiB)

Reviewers: mp+225665_code.launchpad.net,

Message:
Please take a look.

Description:
Fix quickstart for Juju 1.20.

In the addresses included in the MachineInfo,
replace NetworkScope with Scope, falling back to
NetworkScope for backwards compatibility.

Tests:
make check

QA:
Run quickstart with both 1.18 and 1.20 versions of Juju.

https://code.launchpad.net/~frankban/juju-quickstart/fix-120/+merge/225665

(do not edit description out of merge proposal)

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

Affected files (+44, -24 lines):
   A [revision details]
   M quickstart/__init__.py
   M quickstart/tests/test_app.py
   M quickstart/tests/test_watchers.py
   M quickstart/watchers.py

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision:
<email address hidden>
+New revision:
<email address hidden>

Index: quickstart/__init__.py
=== modified file 'quickstart/__init__.py'
--- quickstart/__init__.py 2014-06-17 16:07:50 +0000
+++ quickstart/__init__.py 2014-07-04 13:16:42 +0000
@@ -45,7 +45,7 @@
  Once Juju has been installed, the command can also be run as a juju plugin,
  without the hyphen ("juju quickstart").
  """
-VERSION = (1, 4, 0)
+VERSION = (1, 4, 1)

def get_version():

Index: quickstart/watchers.py
=== modified file 'quickstart/watchers.py'
--- quickstart/watchers.py 2014-04-04 15:49:16 +0000
+++ quickstart/watchers.py 2014-07-04 13:09:11 +0000
@@ -35,23 +35,23 @@

          [
              {'NetworkName': '',
- 'NetworkScope': 'public',
+ 'Scope': 'public',
               'Type': 'hostname',
               'Value': 'eu-west-1.compute.example.com'},
              {'NetworkName': '',
- 'NetworkScope': 'local-cloud',
+ 'Scope': 'local-cloud',
               'Type': 'hostname',
               'Value': 'eu-west-1.example.internal'},
              {'NetworkName': '',
- 'NetworkScope': 'public',
+ 'Scope': 'public',
               'Type': 'ipv4',
               'Value': '444.222.444.222'},
              {'NetworkName': '',
- 'NetworkScope': 'local-cloud',
+ 'Scope': 'local-cloud',
               'Type': 'ipv4',
               'Value': '10.42.47.10'},
              {'NetworkName': '',
- 'NetworkScope': '',
+ 'Scope': '',
               'Type': 'ipv6',
               'Value': 'fe80::92b8:d0ff:fe94:8f8c'},
          ]
@@ -60,11 +60,11 @@

          [
              {'NetworkName': '',
- 'NetworkScope': '',
+ 'Scope': '',
               'Type': 'ipv4',
               'Value': '10.0.3.42'},
              {'NetworkName': '',
- 'NetworkScope': '',
+ 'Scope': '',
               'Type': 'ipv6',
               'Value': 'fe80::216:3eff:fefd:787e'},
          ]
@@ -79,7 +79,8 @@
          value = address['Value']
          # Exclude empty values and ipv6 addresses.
          if value and (address['Type'] != IPV6_ADDRESS):
- scope = add...

Reviewers: mp+225665_code.launchpad.net,

Message:
Please take a look.

Description:
Fix quickstart for Juju 1.20.

In the addresses included in the MachineInfo,
replace NetworkScope with Scope, falling back to
NetworkScope for backwards compatibility.

Tests:
make check

QA:
Run quickstart with both 1.18 and 1.20 versions of Juju.

https://code.launchpad.net/~frankban/juju-quickstart/fix-120/+merge/225665

(do not edit description out of merge proposal)

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

Affected files (+44, -24 lines):
   A [revision details]
   M quickstart/__init__.py
   M quickstart/tests/test_app.py
   M quickstart/tests/test_watchers.py
   M quickstart/watchers.py

Index: [revision details]
=== added file '[revision details]'
--- [revision details]	2012-01-01 00:00:00 +0000
+++ [revision details]	2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision:  
francesco.banconi@canonical.com-20140704122913-ymt58wzjj7s20mt2
+New revision:  
francesco.banconi@canonical.com-20140704131642-pkeyplljmqnbl62x

Index: quickstart/__init__.py
=== modified file 'quickstart/__init__.py'
--- quickstart/__init__.py	2014-06-17 16:07:50 +0000
+++ quickstart/__init__.py	2014-07-04 13:16:42 +0000
@@ -45,7 +45,7 @@
  Once Juju has been installed, the command can also be run as a juju plugin,
  without the hyphen ("juju quickstart").
  """
-VERSION = (1, 4, 0)
+VERSION = (1, 4, 1)

def get_version():

Index: quickstart/watchers.py
=== modified file 'quickstart/watchers.py'
--- quickstart/watchers.py	2014-04-04 15:49:16 +0000
+++ quickstart/watchers.py	2014-07-04 13:09:11 +0000
@@ -35,23 +35,23 @@

[
              {'NetworkName': '',
-             'NetworkScope': 'public',
+             'Scope': 'public',
               'Type': 'hostname',
               'Value': 'eu-west-1.compute.example.com'},
              {'NetworkName': '',
-             'NetworkScope': 'local-cloud',
+             'Scope': 'local-cloud',
               'Type': 'hostname',
               'Value': 'eu-west-1.example.internal'},
              {'NetworkName': '',
-             'NetworkScope': 'public',
+             'Scope': 'public',
               'Type': 'ipv4',
               'Value': '444.222.444.222'},
              {'NetworkName': '',
-             'NetworkScope': 'local-cloud',
+             'Scope': 'local-cloud',
               'Type': 'ipv4',
               'Value': '10.42.47.10'},
              {'NetworkName': '',
-             'NetworkScope': '',
+             'Scope': '',
               'Type': 'ipv6',
               'Value': 'fe80::92b8:d0ff:fe94:8f8c'},
          ]
@@ -60,11 +60,11 @@

[
              {'NetworkName': '',
-             'NetworkScope': '',
+             'Scope': '',
               'Type': 'ipv4',
               'Value': '10.0.3.42'},
              {'NetworkName': '',
-             'NetworkScope': '',
+             'Scope': '',
               'Type': 'ipv6',
               'Value': 'fe80::216:3eff:fefd:787e'},
          ]
@@ -79,7 +79,8 @@
          value = address['Value']
          # Exclude empty values and ipv6 addresses.
          if value and (address['Type'] != IPV6_ADDRESS):
-            scope = address['NetworkScope']
+            # The address scope was called "NetworkScope" prior to juju  
1.20.0.
+            scope = address.get('Scope', address.get('NetworkScope'))
              # If the scope is public then we have found the address.
              if scope == NETWORK_PUBLIC:
                  return value

Index: quickstart/tests/test_app.py
=== modified file 'quickstart/tests/test_app.py'
--- quickstart/tests/test_app.py	2014-07-04 07:49:03 +0000
+++ quickstart/tests/test_app.py	2014-07-04 13:09:11 +0000
@@ -1272,7 +1272,7 @@
          if address is not None:
              data['Addresses'] = [{
                  'NetworkName': '',
-                'NetworkScope': 'public',
+                'Scope': 'public',
                  'Type': 'hostname',
                  'Value': address,
              }]

Index: quickstart/tests/test_watchers.py
=== modified file 'quickstart/tests/test_watchers.py'
--- quickstart/tests/test_watchers.py	2014-04-07 13:24:33 +0000
+++ quickstart/tests/test_watchers.py	2014-07-04 13:09:11 +0000
@@ -29,33 +29,33 @@
  # Define addresses to be used in tests.
  cloud_addresses = [
      {'NetworkName': '',
-     'NetworkScope': 'public',
+     'Scope': 'public',
       'Type': 'hostname',
       'Value': 'eu-west-1.compute.example.com'},
      {'NetworkName': '',
-     'NetworkScope': 'local-cloud',
+     'Scope': 'local-cloud',
       'Type': 'hostname',
       'Value': 'eu-west-1.example.internal'},
      {'NetworkName': '',
-     'NetworkScope': 'public',
+     'Scope': 'public',
       'Type': 'ipv4',
       'Value': '444.222.444.222'},
      {'NetworkName': '',
-     'NetworkScope': 'local-cloud',
+     'Scope': 'local-cloud',
       'Type': 'ipv4',
       'Value': '10.42.47.10'},
      {'NetworkName': '',
-     'NetworkScope': '',
+     'Scope': '',
       'Type': 'ipv6',
       'Value': 'fe80::92b8:d0ff:fe94:8f8c'},
  ]
  container_addresses = [
      {'NetworkName': '',
-     'NetworkScope': '',
+     'Scope': '',
       'Type': 'ipv4',
       'Value': '10.0.3.42'},
      {'NetworkName': '',
-     'NetworkScope': '',
+     'Scope': '',
       'Type': 'ipv6',
       'Value': 'fe80::216:3eff:fefd:787e'},
  ]
@@ -71,11 +71,11 @@
          # None is returned if a cloud machine public address is not  
available.
          addresses = [
              {'NetworkName': '',
-             'NetworkScope': 'local-cloud',
+             'Scope': 'local-cloud',
               'Type': 'hostname',
               'Value': 'eu-west-1.example.internal'},
              {'NetworkName': '',
-             'NetworkScope': 'local-cloud',
+             'Scope': 'local-cloud',
               'Type': 'ipv4',
               'Value': '10.42.47.10'},
          ]
@@ -85,7 +85,7 @@
          # None is returned if an LXC public address is not available.
          addresses = [{
              'NetworkName': '',
-            'NetworkScope': '',
+            'Scope': '',
              'Type': 'ipv6',
              'Value': 'fe80::216:3eff:fefd:787e',
          }]
@@ -95,11 +95,11 @@
          # None is returned if the public address has no value.
          addresses = [
              {'NetworkName': '',
-             'NetworkScope': 'local-cloud',
+             'Scope': 'local-cloud',
               'Type': 'hostname',
               'Value': 'eu-west-1.example.internal'},
              {'NetworkName': '',
-             'NetworkScope': 'public',
+             'Scope': 'public',
               'Type': 'ipv4',
               'Value': ''},
          ]
@@ -115,15 +115,32 @@
          public_address =  
watchers.retrieve_public_adddress(container_addresses)
          self.assertEqual('10.0.3.42', public_address)

+    def test_old_juju_version(self):
+        # The public address is properly returned when using Juju < 1.20.
+        # Prior to Juju 1.20, the MachineInfo field for the address scope  
was
+        # called "NetworkScope".
+        addresses = [
+            {'NetworkName': '',
+             'NetworkScope': 'public',
+             'Type': 'hostname',
+             'Value': 'eu-west-1.compute.example.com'},
+            {'NetworkName': '',
+             'NetworkScope': 'local-cloud',
+             'Type': 'hostname',
+             'Value': 'eu-west-1.example.internal'},
+        ]
+        public_address = watchers.retrieve_public_adddress(addresses)
+        self.assertEqual('eu-west-1.compute.example.com', public_address)
+
      def test_last_unknown_address(self):
          # If the scope of multiple addresses is unknown, the last one is  
taken.
          addresses = [
              {'NetworkName': '',
-             'NetworkScope': '',
+             'Scope': '',
               'Type': 'ipv4',
               'Value': '10.0.3.42'},
              {'NetworkName': '',
-             'NetworkScope': '',
+             'Scope': '',
               'Type': 'ipv4',
               'Value': '10.0.3.47'},
          ]

Revision history for this message

Brad Crittenden (bac) wrote on 2014-07-04:

#

LGTM

https://codereview.appspot.com/111860043/diff/1/quickstart/watchers.py
File quickstart/watchers.py (right):

https://codereview.appspot.com/111860043/diff/1/quickstart/watchers.py#newcode83
quickstart/watchers.py:83: scope = address.get('Scope',
address.get('NetworkScope'))
Nice fix.