Code review comment for lp:~frankban/juju-quickstart/changeset-prep

Revision history for this message
Francesco Banconi (frankban) wrote :

Reviewers: mp+257387_code.launchpad.net,

Message:
Please take a look.

Description:
Some small changes in preparation for change sets.

Extend the services method of the bundle model to
include the number of units for each service.

Use the newer jujubundlelib.

https://code.launchpad.net/~frankban/juju-quickstart/changeset-prep/+merge/257387

(do not edit description out of merge proposal)

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

Affected files (+19, -10 lines):
   A [revision details]
   M quickstart/manage.py
   M quickstart/models/bundles.py
   M quickstart/tests/models/test_bundles.py
   M tox.ini

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: tox.ini
=== modified file 'tox.ini'
--- tox.ini 2015-04-23 09:00:26 +0000
+++ tox.ini 2015-04-24 13:31:28 +0000
@@ -72,7 +72,7 @@
      # See https://launchpad.net/~juju/+archive/ubuntu/stable.
      websocket-client==0.18.0
      jujuclient==0.50.1
- jujubundlelib==0.1.3
+ jujubundlelib==0.1.4
      urwid==1.2.1
      # The distribution PyYAML requirement is used in this case.

@@ -82,7 +82,7 @@
      # Ubuntu 14.04 (trusty) distro dependencies.
      websocket-client==0.12.0
      jujuclient==0.17.5
- jujubundlelib==0.1.3
+ jujubundlelib==0.1.4
      PyYAML==3.10
      urwid==1.1.1

@@ -92,7 +92,7 @@
      # Ubuntu 14.10 (utopic) distro dependencies.
      websocket-client==0.12.0
      jujuclient==0.17.5
- jujubundlelib==0.1.3
+ jujubundlelib==0.1.4
      PyYAML==3.11
      urwid==1.2.1

@@ -102,7 +102,7 @@
      # Ubuntu 15.04 (vivid) distro dependencies.
      websocket-client==0.18.0
      jujuclient==0.18.5
- jujubundlelib==0.1.3
+ jujubundlelib==0.1.4
      PyYAML==3.11
      urwid==1.2.1

Index: quickstart/manage.py
=== modified file 'quickstart/manage.py'
--- quickstart/manage.py 2015-04-21 10:22:46 +0000
+++ quickstart/manage.py 2015-04-24 13:31:28 +0000
@@ -575,7 +575,7 @@

      # Handle bundle deployment.
      if options.bundle_source is not None:
- services = ', '.join(options.bundle.services())
+ services = ', '.join(options.bundle.services().keys())
          print('requesting a deployment of {} with the following
services:\n'
                ' {}'.format(options.bundle, services))
          if options.bundle.reference is not None:

Index: quickstart/models/bundles.py
=== modified file 'quickstart/models/bundles.py'
--- quickstart/models/bundles.py 2015-04-23 12:09:29 +0000
+++ quickstart/models/bundles.py 2015-04-24 13:31:28 +0000
@@ -106,11 +106,15 @@
          return serializers.yaml_dump({'bundle': self.data})

      def services(self):
- """Return a list of service names included in the bundle.
+ """Return an ordered dict mapping services and their number of
units.

- Service names are returned in alphabetical order.
+ In the dict, service names are returned in alphabetical order.
          """
- return sorted(self.data['services'].keys())
+ services = self.data['services']
+ result = collections.OrderedDict()
+ for service_name in sorted(services.keys()):
+ result[service_name] = services[service_name]['num_units']
+ return result

  def from_source(source, name=None):

Index: quickstart/tests/models/test_bundles.py
=== modified file 'quickstart/tests/models/test_bundles.py'
--- quickstart/tests/models/test_bundles.py 2015-04-23 12:13:27 +0000
+++ quickstart/tests/models/test_bundles.py 2015-04-24 13:31:28 +0000
@@ -18,6 +18,7 @@

  from __future__ import unicode_literals

+import collections
  import json
  import unittest

@@ -115,8 +116,10 @@
          self.assertEqual(expected_content, self.bundle.serialize_legacy())

      def test_services(self):
- # Bundle services can be easily retrieved.
- self.assertEqual(['mysql', 'wordpress'], self.bundle.services())
+ # Bundle services and their number of units can be easily
retrieved.
+ expected_services = collections.OrderedDict(
+ [('mysql', 0), ('wordpress', 1)])
+ self.assertEqual(expected_services, self.bundle.services())

  class TestFromSource(

« Back to merge proposal