Merge lp:~frankban/juju-quickstart/fix-num-units into lp:juju-quickstart

[Francesco Banconi]

Fix key error: num_units not defined in services.

https://codereview.appspot.com/235490043/

[Francesco Banconi (frankban)]

Reviewers: mp+258972_code.launchpad.net,

Message:
Please take a look.

Description:
Fix key error: num_units not defined in services.

https://code.launchpad.net/~frankban/juju-quickstart/fix-num-units/+merge/258972

(do not edit description out of merge proposal)

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

Affected files (+27, -4 lines):
   A [revision details]
   M quickstart/__init__.py
   M quickstart/models/bundles.py
   M quickstart/tests/functional/test_functional.py
   M quickstart/tests/models/test_bundles.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 2015-05-11 10:55:51 +0000
+++ quickstart/__init__.py 2015-05-13 08:27:30 +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 = (2, 1, 0)
+VERSION = (2, 1, 1)

  def get_version():

Index: quickstart/models/bundles.py
=== modified file 'quickstart/models/bundles.py'
--- quickstart/models/bundles.py 2015-04-24 13:31:28 +0000
+++ quickstart/models/bundles.py 2015-05-13 08:31:51 +0000
@@ -113,7 +113,7 @@
          services = self.data['services']
          result = collections.OrderedDict()
          for service_name in sorted(services.keys()):
- result[service_name] = services[service_name]['num_units']
+ result[service_name] = services[service_name].get('num_units',
0)
          return result

Index: quickstart/tests/functional/test_functional.py
=== modified file 'quickstart/tests/functional/test_functional.py'
--- quickstart/tests/functional/test_functional.py 2015-02-26 19:12:17 +0000
+++ quickstart/tests/functional/test_functional.py 2015-05-13 08:31:51 +0000
@@ -166,8 +166,17 @@

      def test_bundle_deployment(self):
          # The application can be used to deploy bundles.
- retcode, output, error = run_quickstart(
- self.env_name, 'mediawiki-single')
+ self._check_bundle('mediawiki-single/7')
+
+ def test_user_owned_bundle_deployment(self):
+ # The application can be used to deploy user owned bundles.
+ # The kubernetes bundle also includes subordinate services not
+ # including the "num_units" field.
+ self._check_bundle('u/kubernetes/kubernetes-cluster/3')
+
+ def _check_bundle(self, bundle_name):
+ """Ensure the given bundle name can be properly deployed."""
+ retcode, output, error = run_quickstart(self.env_name, bundle_name)
          self.assertEqual(0, retcode)
          self.assertIn('bundle deployment request accepted', output)
          self.assertEqual('', error)

Index: quickstart/tests/models/test_bundles.py
=== modified file 'quickstart/tests/models/test_bundles.py'
--- quickstart/tests/models/test_bundles.py 2015-04-28 15:25:14 +0000
+++ quic...

[Martin Hilton (martin-hilton)]

LGTM, but make sure that the default 0 is correct for all cases.

https://codereview.appspot.com/235490043/diff/1/quickstart/models/bundles.py
File quickstart/models/bundles.py (left):

https://codereview.appspot.com/235490043/diff/1/quickstart/models/bundles.py#oldcode116
quickstart/models/bundles.py:116: result[service_name] =
services[service_name]['num_units']
Is 0 always a good default? I thought for subordinates it should be 0
but for normal charms it should be 1.

https://codereview.appspot.com/235490043/

[lp:~frankban/juju-quickstart/fix-num-units]

136. By Francesco Banconi on 2015-05-13

None units

[Francesco Banconi (frankban)]

Please take a look.

https://codereview.appspot.com/235490043/

[Uros Jovanovic (uros-jovanovic)]

On 2015/05/13 08:49:04, frankban wrote:
> Please take a look.

LGTM now.

https://codereview.appspot.com/235490043/

[Francesco Banconi (frankban)]

*** Submitted:

Fix key error: num_units not defined in services.

R=martin.hilton, uros.jovanovic1
CC=
https://codereview.appspot.com/235490043

https://codereview.appspot.com/235490043/

[Francesco Banconi (frankban)]

Thanks for the reviews!

https://codereview.appspot.com/235490043/

=== modified file 'quickstart/__init__.py'
--- quickstart/__init__.py	2015-05-11 10:55:51 +0000
+++ quickstart/__init__.py	2015-05-13 08:46:43 +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 = (2, 1, 0)
+VERSION = (2, 1, 1)
 
 
 def get_version():

=== modified file 'quickstart/models/bundles.py'
--- quickstart/models/bundles.py	2015-04-24 13:31:28 +0000
+++ quickstart/models/bundles.py	2015-05-13 08:46:43 +0000
@@ -113,7 +113,7 @@
         services = self.data['services']
         result = collections.OrderedDict()
         for service_name in sorted(services.keys()):
-            result[service_name] = services[service_name]['num_units']
+            result[service_name] = services[service_name].get('num_units')
         return result
 
 

=== modified file 'quickstart/tests/functional/test_functional.py'
--- quickstart/tests/functional/test_functional.py	2015-02-26 19:12:17 +0000
+++ quickstart/tests/functional/test_functional.py	2015-05-13 08:46:43 +0000
@@ -166,8 +166,17 @@
 
     def test_bundle_deployment(self):
         # The application can be used to deploy bundles.
-        retcode, output, error = run_quickstart(
-            self.env_name, 'mediawiki-single')
+        self._check_bundle('mediawiki-single/7')
+
+    def test_user_owned_bundle_deployment(self):
+        # The application can be used to deploy user owned bundles.
+        # The kubernetes bundle also includes subordinate services not
+        # including the "num_units" field.
+        self._check_bundle('u/kubernetes/kubernetes-cluster/3')
+
+    def _check_bundle(self, bundle_name):
+        """Ensure the given bundle name can be properly deployed."""
+        retcode, output, error = run_quickstart(self.env_name, bundle_name)
         self.assertEqual(0, retcode)
         self.assertIn('bundle deployment request accepted', output)
         self.assertEqual('', error)

=== modified file 'quickstart/tests/models/test_bundles.py'
--- quickstart/tests/models/test_bundles.py	2015-04-28 15:25:14 +0000
+++ quickstart/tests/models/test_bundles.py	2015-05-13 08:46:43 +0000
@@ -121,6 +121,18 @@
             [('mysql', 0), ('wordpress', 1)])
         self.assertEqual(expected_services, self.bundle.services())
 
+    def test_services_subordinate_units(self):
+        # The number of units, if not defined, is None.
+        bundle = bundles.Bundle({
+            'services': {
+                'logger': {'charm': 'cs:trusty/logger-42'},
+                'django': {'charm': 'django', 'num_units': 2},
+            },
+        })
+        expected_services = collections.OrderedDict(
+            [('django', 2), ('logger', None)])
+        self.assertEqual(expected_services, bundle.services())
+
 
 class TestFromSource(
         helpers.BundleFileTestsMixin, helpers.UrlReadTestsMixin,