Merge lp:~frankban/juju-quickstart/add-settings into lp:juju-quickstart

[Francesco Banconi]

Put constant parameters in a separate module.

Added a settings module where to store constants
reused in different parts of the application.

This is just a mechanical branch and it does
not include other changes.

https://codereview.appspot.com/26060043/

[Francesco Banconi (frankban)]

Reviewers: mp+195057_code.launchpad.net,

Message:
Please take a look.

Description:
Put constant parameters in a separate module.

Added a settings module where to store constants
reused in different parts of the application.

This is just a mechanical branch and it does
not include other changes.

https://code.launchpad.net/~frankban/juju-quickstart/add-settings/+merge/195057

(do not edit description out of merge proposal)

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

Affected files (+73, -29 lines):
   A [revision details]
   M quickstart/app.py
   M quickstart/manage.py
   A quickstart/settings.py
   M quickstart/tests/test_app.py
   M quickstart/tests/test_manage.py
   M quickstart/tests/test_utils.py
   M quickstart/utils.py

[Gary Poster (gary)]

LGTM. Were you driven to do this now because of circular import fun?

https://codereview.appspot.com/26060043/

[Francesco Banconi (frankban)]

*** Submitted:

Put constant parameters in a separate module.

Added a settings module where to store constants
reused in different parts of the application.

This is just a mechanical branch and it does
not include other changes.

R=gary.poster
CC=
https://codereview.appspot.com/26060043

https://codereview.appspot.com/26060043/

[Francesco Banconi (frankban)]

On 2013/11/13 13:37:51, gary.poster wrote:
> LGTM. Were you driven to do this now because of circular import fun?

Well, I did this in prevision of that possibility, and also to have
a single place where looking for names which can be useful and used
in different parts of the code base.

Thank you!

https://codereview.appspot.com/26060043/

=== modified file 'quickstart/app.py'
--- quickstart/app.py	2013-11-12 15:55:34 +0000
+++ quickstart/app.py	2013-11-13 13:18:35 +0000
@@ -25,15 +25,11 @@
 
 from quickstart import (
     juju,
+    settings,
     utils,
 )
 
 
-# The default Juju GUI charm URL to use when it is not possible to retrieve it
-# from the charmworld API, e.g. due to temporary connection/charmworld errors.
-DEFAULT_CHARM_URL = 'cs:precise/juju-gui-79'
-
-
 class ProgramExit(Exception):
     """An error occurred while setting up the Juju environment.
 
@@ -133,7 +129,7 @@
         except (IOError, ValueError) as err:
             msg = 'unable to retrieve the Juju GUI charm URL from the API: {}'
             logging.warn(msg.format(err))
-            charm_url = DEFAULT_CHARM_URL
+            charm_url = settings.DEFAULT_CHARM_URL
     print('charm URL: {}'.format(charm_url))
     try:
         env.deploy(service_name, charm_url, to=0)

=== modified file 'quickstart/manage.py'
--- quickstart/manage.py	2013-11-12 17:01:34 +0000
+++ quickstart/manage.py	2013-11-13 13:18:35 +0000
@@ -25,12 +25,11 @@
 import quickstart
 from quickstart import (
     app,
+    settings,
     utils,
 )
 
 
-juju_home = os.getenv('JUJU_HOME', '~/.juju')
-description = 'set up a Juju environment (including the GUI) in very few steps'
 version = quickstart.get_version()
 
 
@@ -38,7 +37,7 @@
     """A customized argparse action that just shows a description."""
 
     def __call__(self, parser, *args, **kwargs):
-        print(description)
+        print(settings.DESCRIPTION)
         parser.exit()
 
 
@@ -180,8 +179,8 @@
              'required if the bundle YAML/JSON only contains one bundle. This '
              'option is ignored if the bundle file is not specified')
     parser.add_argument(
-        '--environments-file',
-        default=os.path.join(juju_home, 'environments.yaml'), dest='env_file',
+        '--environments-file', dest='env_file',
+        default=os.path.join(settings.JUJU_HOME, 'environments.yaml'),
         help='The path to the Juju environments YAML file (%(default)s)')
     parser.add_argument(
         '--gui-charm-url', dest='charm_url',
@@ -225,9 +224,9 @@
     print('connecting to {}'.format(api_url))
     env = app.connect(api_url, options.admin_secret)
     print('requesting Juju GUI deployment')
-    app.deploy_gui(env, 'juju-gui', charm_url=options.charm_url)
+    app.deploy_gui(env, settings.JUJU_GUI_NAME, charm_url=options.charm_url)
     print('Juju GUI deployment request accepted')
-    address = app.watch(env, 'juju-gui')
+    address = app.watch(env, settings.JUJU_GUI_NAME)
     url = 'https://{}'.format(address)
     print('url: {}\npassword: {}'.format(url, options.admin_secret))
 

=== added file 'quickstart/settings.py'
--- quickstart/settings.py	1970-01-01 00:00:00 +0000
+++ quickstart/settings.py	2013-11-13 13:18:35 +0000
@@ -0,0 +1,36 @@
+# This file is part of the Juju Quickstart Plugin, which lets users set up a
+# Juju environment in very few steps (https://launchpad.net/juju-quickstart).
+# Copyright (C) 2013 Canonical Ltd.
+#
+# This program is free software: you can redistribute it and/or modify it under
+# the terms of the GNU Affero General Public License version 3, as published by
+# the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranties of MERCHANTABILITY,
+# SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Juju Quickstart settings."""
+
+import os
+
+
+# The URL containing information about the last Juju GUI charm version.
+CHARMWORLD_API = 'http://manage.jujucharms.com/api/3/charm/precise/juju-gui'
+
+# The default Juju GUI charm URL to use when it is not possible to retrieve it
+# from the charmworld API, e.g. due to temporary connection/charmworld errors.
+DEFAULT_CHARM_URL = 'cs:precise/juju-gui-79'
+
+# The quickstart app short description.
+DESCRIPTION = 'set up a Juju environment (including the GUI) in very few steps'
+
+# Retrieve the current juju-core home.
+JUJU_HOME = os.getenv('JUJU_HOME', '~/.juju')
+
+# The name of the Juju GUI service.
+JUJU_GUI_NAME = 'juju-gui'

=== modified file 'quickstart/tests/test_app.py'
--- quickstart/tests/test_app.py	2013-11-06 15:12:43 +0000
+++ quickstart/tests/test_app.py	2013-11-13 13:18:35 +0000
@@ -24,7 +24,10 @@
 import mock
 import yaml
 
-from quickstart import app
+from quickstart import (
+    app,
+    settings,
+)
 from quickstart.tests import helpers
 
 
@@ -282,11 +285,11 @@
             with helpers.assert_logs([log], level='warn'):
                 app.deploy_gui(env, 'my-gui')
         env.assert_has_calls([
-            mock.call.deploy('my-gui', app.DEFAULT_CHARM_URL, to=0),
+            mock.call.deploy('my-gui', settings.DEFAULT_CHARM_URL, to=0),
             mock.call.expose('my-gui')
         ])
         mock_print.assert_called_once_with(
-            'charm URL: {}'.format(app.DEFAULT_CHARM_URL))
+            'charm URL: {}'.format(settings.DEFAULT_CHARM_URL))
 
     def test_deployment_provided_charm_url(self, mock_print):
         # The function correctly deploys and exposes the service using a user

=== modified file 'quickstart/tests/test_manage.py'
--- quickstart/tests/test_manage.py	2013-11-12 15:34:15 +0000
+++ quickstart/tests/test_manage.py	2013-11-13 13:18:35 +0000
@@ -27,7 +27,10 @@
 import yaml
 
 import quickstart
-from quickstart import manage
+from quickstart import (
+    manage,
+    settings,
+)
 from quickstart.tests import helpers
 
 
@@ -44,7 +47,7 @@
         # The action just prints the description and exits.
         args = self.parser.parse_args(['--test'])
         self.assertIsNone(args.test)
-        mock_print.assert_called_once_with(manage.description)
+        mock_print.assert_called_once_with(settings.DESCRIPTION)
         mock_exit.assert_called_once_with(0)
 
 
@@ -293,7 +296,7 @@
         # The program description is properly printed out as required by juju.
         with mock.patch('__builtin__.print') as mock_print:
             self.call_setup(['--description'])
-        mock_print.assert_called_once_with(manage.description)
+        mock_print.assert_called_once_with(settings.DESCRIPTION)
 
     def test_version(self):
         # The program version is properly printed to stderr.
@@ -353,8 +356,10 @@
         mock_app.connect.assert_called_once_with(
             mock_app.get_api_url(), options.admin_secret)
         mock_app.deploy_gui.assert_called_once_with(
-            mock_app.connect(), 'juju-gui', charm_url=options.charm_url)
-        mock_app.watch.assert_called_once_with(mock_app.connect(), 'juju-gui')
+            mock_app.connect(), settings.JUJU_GUI_NAME,
+            charm_url=options.charm_url)
+        mock_app.watch.assert_called_once_with(
+            mock_app.connect(), settings.JUJU_GUI_NAME)
         mock_open.assert_called_once_with(
             'https://{}'.format(mock_app.watch()))
         self.assertFalse(mock_app.deploy_bundle.called)

=== modified file 'quickstart/tests/test_utils.py'
--- quickstart/tests/test_utils.py	2013-11-12 19:45:11 +0000
+++ quickstart/tests/test_utils.py	2013-11-13 13:18:35 +0000
@@ -25,7 +25,10 @@
 import mock
 import yaml
 
-from quickstart import utils
+from quickstart import (
+    settings,
+    utils,
+)
 from quickstart.tests import helpers
 
 
@@ -105,14 +108,14 @@
         with self.patch_urlread(contents=contents) as mock_urlread:
             charm_url = utils.get_charm_url()
         self.assertEqual('cs:precise/juju-gui-42', charm_url)
-        mock_urlread.assert_called_once_with(utils.CHARMWORLD_API)
+        mock_urlread.assert_called_once_with(settings.CHARMWORLD_API)
 
     def test_io_error(self):
         # IOErrors are properly propagated.
         with self.patch_urlread(error=True) as mock_urlread:
             with self.assertRaises(IOError) as context_manager:
                 utils.get_charm_url()
-        mock_urlread.assert_called_once_with(utils.CHARMWORLD_API)
+        mock_urlread.assert_called_once_with(settings.CHARMWORLD_API)
         self.assertEqual('bad wolf', str(context_manager.exception))
 
     def test_value_error(self):
@@ -121,7 +124,7 @@
         with self.patch_urlread(contents=contents) as mock_urlread:
             with self.assertRaises(ValueError) as context_manager:
                 utils.get_charm_url()
-        mock_urlread.assert_called_once_with(utils.CHARMWORLD_API)
+        mock_urlread.assert_called_once_with(settings.CHARMWORLD_API)
         self.assertEqual(
             'unable to find the charm URL', str(context_manager.exception))
 

=== modified file 'quickstart/utils.py'
--- quickstart/utils.py	2013-11-12 19:45:11 +0000
+++ quickstart/utils.py	2013-11-13 13:18:35 +0000
@@ -29,10 +29,10 @@
 
 import yaml
 
+from quickstart import settings
+
 # Compile the regular expression used to parse the "juju switch" output.
 _juju_switch_expression = re.compile(r'Current environment: "([\w-]+)"\n')
-# Define the URL containing information about the last Juju GUI charm version.
-CHARMWORLD_API = 'http://manage.jujucharms.com/api/3/charm/precise/juju-gui'
 
 
 def call(*args):
@@ -75,7 +75,7 @@
     Raise an IOError if any problems occur connecting to the API endpoint.
     Raise a ValueError if the API returns invalid data.
     """
-    charm_info = json.loads(urlread(CHARMWORLD_API))
+    charm_info = json.loads(urlread(settings.CHARMWORLD_API))
     charm_url = charm_info.get('charm', {}).get('url')
     if charm_url is None:
         raise ValueError('unable to find the charm URL')
@@ -162,7 +162,7 @@
     if not bundle_services:
         msg = 'bundle {} does not include any services'.format(bundle_name)
         raise ValueError(msg)
-    if 'juju-gui' in bundle_services:
+    if settings.JUJU_GUI_NAME in bundle_services:
         raise ValueError('bundle {} contains an instance of juju-gui. '
                          'quickstart will install the latest version of the '
                          'Juju GUI automatically, please remove juju-gui from '