Merge lp:~frankban/juju-quickstart/minor-fixes-0 into lp:juju-quickstart

[Francesco Banconi]

Minor fixes to code comments and documentation

Also fixed a possible encoding problem in error handling.

The get_admin_secret function and its test case are just moved, no
need to re-review.

Tests: `make check`.
No QA.

https://codereview.appspot.com/58680043/

[Francesco Banconi (frankban)]

Reviewers: mp+203942_code.launchpad.net,

Message:
Please take a look.

Description:
Minor fixes to code comments and documentation

Also fixed a possible encoding problem in error handling.

The get_admin_secret function and its test case are just moved, no
need to re-review.

Tests: `make check`.
No QA.

https://code.launchpad.net/~frankban/juju-quickstart/minor-fixes-0/+merge/203942

(do not edit description out of merge proposal)

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

Affected files (+58, -48 lines):
   M HACKING.rst
   A [revision details]
   M quickstart/app.py
   M quickstart/manage.py
   M quickstart/tests/test_app.py
   M quickstart/tests/test_manage.py

[Brad Crittenden (bac)]

LGTM

https://codereview.appspot.com/58680043/

[Francesco Banconi (frankban)]

*** Submitted:

Minor fixes to code comments and documentation

Also fixed a possible encoding problem in error handling.

The get_admin_secret function and its test case are just moved, no
need to re-review.

Tests: `make check`.
No QA.

R=bac
CC=
https://codereview.appspot.com/58680043

https://codereview.appspot.com/58680043/

=== modified file 'HACKING.rst'
--- HACKING.rst	2013-11-20 08:58:02 +0000
+++ HACKING.rst	2014-01-30 12:27:24 +0000
@@ -110,6 +110,14 @@
 available. They are available in saucy, and they are also stored in our PPA in
 order to support previous Ubuntu releases.
 
+Creating PyPI releases
+~~~~~~~~~~~~~~~~~~~~~~
+
+Juju Quickstart is present on PyPI: see
+<https://pypi.python.org/pypi/juju-quickstart>.
+It is possible to register and upload a new release on PyPI by just running
+``make release`` and providing your PyPI credentials.
+
 Updating application and test dependencies
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
 
=== modified file 'quickstart/app.py'
--- quickstart/app.py	2014-01-29 14:25:35 +0000
+++ quickstart/app.py	2014-01-30 12:27:24 +0000
@@ -288,6 +288,28 @@
     raise ProgramExit('the state server is not ready:\n{}'.format(details))
 
 
+def get_admin_secret(env_name, juju_home):
+    """Read the admin-secret from the generated environment file.
+
+    At bootstrap, juju (v1.16 and later) writes the admin-secret to a
+    generated file located in JUJU_HOME.  Return the value.
+    Raise a ValueError if:
+        - the environment file is not found;
+        - the environment file contents are not parsable by YAML;
+        - the YAML contents are not properly structured;
+        - the admin-secret is not found.
+    """
+    filename = '{}.jenv'.format(env_name)
+    juju_env_file = os.path.expanduser(
+        os.path.join(juju_home, 'environments', filename))
+    jenv_db = envs.load_generated(juju_env_file)
+    try:
+        return jenv_db['admin-secret']
+    except KeyError:
+        msg = 'admin-secret not found in {}'.format(juju_env_file)
+        raise ValueError(msg.encode('utf-8'))
+
+
 def get_api_url(env_name):
     """Return a Juju API URL for the given environment name.
 
@@ -497,25 +519,3 @@
         env.deploy_bundle(bundle_yaml, name=bundle_name, bundle_id=bundle_id)
     except jujuclient.EnvError as err:
         raise ProgramExit('bad API server response: {}'.format(err.message))
-
-
-def get_admin_secret(env_name, juju_home):
-    """Read the admin-secret from the generated environment file.
-
-    At bootstrap, juju (v1.16 and later) writes the admin-secret to a
-    generated file located in JUJU_HOME.  Return the value.
-    Raise a ValueError if:
-        - the environment file is not found;
-        - the environment file contents are not parsable by YAML;
-        - the YAML contents are not properly structured;
-        - the admin-secret is not found.
-    """
-    filename = '{}.jenv'.format(env_name)
-    juju_env_file = os.path.expanduser(
-        os.path.join(juju_home, 'environments', filename))
-    jenv_db = envs.load_generated(juju_env_file)
-    try:
-        return jenv_db['admin-secret']
-    except KeyError:
-        msg = 'admin-secret not found in {}'.format(juju_env_file)
-        raise ValueError(msg.encode('utf-8'))
 
=== modified file 'quickstart/manage.py'
--- quickstart/manage.py	2014-01-29 14:25:35 +0000
+++ quickstart/manage.py	2014-01-30 12:27:24 +0000
@@ -392,17 +392,17 @@
     already_bootstrapped, bsn_series = app.bootstrap(
         options.env_name, is_local=is_local, debug=options.debug)
 
-    # The admin secret was not in the environments file.  Retrieve it from
-    # the Juju-generated file.
-    # Find the Juju-generated environment file.
+    # Retrieve the admin-secret for the current environment.
     try:
         admin_secret = app.get_admin_secret(
             options.env_name, settings.JUJU_HOME)
     except ValueError as err:
         admin_secret = options.admin_secret
         if admin_secret is None:
-            msg = err.message + ' or {}.'.format(options.env_file)
-            raise app.ProgramExit(msg.encode('utf-8'))
+            # The admin-secret cannot be found in the jenv file and is not
+            # explicitly specified in the environments.yaml file.
+            msg = b'{} or {}'.format(err, options.env_file.encode('utf-8'))
+            raise app.ProgramExit(msg)
 
     print('retrieving the Juju API address')
     api_url = app.get_api_url(options.env_name)
 
=== modified file 'quickstart/tests/test_app.py'
--- quickstart/tests/test_app.py	2014-01-29 14:25:35 +0000
+++ quickstart/tests/test_app.py	2014-01-30 12:27:24 +0000
@@ -569,6 +569,26 @@
         ] + self.make_status_calls(2))
 
 
+class TestGetAdminSecret(unittest.TestCase):
+
+    def test_no_admin_secret(self):
+        with mock.patch('quickstart.manage.envs.load_generated',
+                        lambda x: {}):
+            with self.assertRaises(ValueError) as exc:
+                app.get_admin_secret('local', '/home/bac/.juju')
+        expected = (
+            u'admin-secret not found in '
+            '/home/bac/.juju/environments/local.jenv')
+        self.assertIn(expected, bytes(exc.exception))
+
+    def test_success(self):
+        expected = 'superchunk'
+        with mock.patch('quickstart.manage.envs.load_generated',
+                        lambda x: {'admin-secret': expected}):
+            secret = app.get_admin_secret('local', '~bac/.juju')
+        self.assertEqual(expected, secret)
+
+
 class TestGetApiUrl(
         helpers.CallTestsMixin, ProgramExitTestsMixin, unittest.TestCase):
 
@@ -1311,23 +1331,3 @@
         with self.assertRaises(ValueError) as context_manager:
             app.deploy_bundle(env, self.yaml, self.name, None)
         self.assertIs(error, context_manager.exception)
-
-
-class TestGetAdminSecret(unittest.TestCase):
-
-    def test_no_admin_secret(self):
-        with mock.patch('quickstart.manage.envs.load_generated',
-                        lambda x: {}):
-            with self.assertRaises(ValueError) as exc:
-                app.get_admin_secret('local', '/home/bac/.juju')
-        expected = (
-            u'admin-secret not found in '
-            '/home/bac/.juju/environments/local.jenv')
-        self.assertIn(expected, bytes(exc.exception))
-
-    def test_success(self):
-        expected = 'superchunk'
-        with mock.patch('quickstart.manage.envs.load_generated',
-                        lambda x: {'admin-secret': expected}):
-            secret = app.get_admin_secret('local', '~bac/.juju')
-        self.assertEqual(expected, secret)
 
=== modified file 'quickstart/tests/test_manage.py'
--- quickstart/tests/test_manage.py	2014-01-29 14:26:43 +0000
+++ quickstart/tests/test_manage.py	2014-01-30 12:27:24 +0000
@@ -770,5 +770,5 @@
             manage.run(options)
         expected = (
             u'admin-secret not found in ~/.juju/environments/local.jenv '
-            'or environments.yaml.')
+            'or environments.yaml')
         self.assertEqual(expected, context.exception.message)