Merge lp:~frankban/juju-quickstart/env-management-1 into lp:juju-quickstart

[Francesco Banconi]

Envs management: code reorganization.

This branch includes code reorg as
a preparation for the upcoming envs
management work.

It's just about mechanically moving code
to the new models package, except for a
minor fix to test_init_environment in
test_manage.py.

https://codereview.appspot.com/37150043/

[Francesco Banconi (frankban)]

Reviewers: mp+197709_code.launchpad.net,

Message:
Please take a look.

Description:
Envs management: code reorganization.

This branch includes code reorg as
a preparation for the upcoming envs
management work.

It's just about mechanically moving code
to the new models package, except for a
minor fix to test_init_environment in
test_manage.py.

https://code.launchpad.net/~frankban/juju-quickstart/env-management-1/+merge/197709

(do not edit description out of merge proposal)

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

Affected files (+269, -202 lines):
   A [revision details]
   M quickstart/manage.py
   A quickstart/models/__init__.py
   M quickstart/models/charms.py
   A quickstart/models/envs.py
   A quickstart/tests/models/__init__.py
   M quickstart/tests/models/test_charms.py
   A quickstart/tests/models/test_envs.py
   M quickstart/tests/test_manage.py
   M quickstart/tests/test_utils.py
   M quickstart/utils.py

[Gary Poster (gary)]

LGTM. Thank you!

https://codereview.appspot.com/37150043/

[Francesco Banconi (frankban)]

*** Submitted:

Envs management: code reorganization.

This branch includes code reorg as
a preparation for the upcoming envs
management work.

It's just about mechanically moving code
to the new models package, except for a
minor fix to test_init_environment in
test_manage.py.

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

https://codereview.appspot.com/37150043/

=== modified file 'quickstart/manage.py'
--- quickstart/manage.py	2013-12-02 21:36:04 +0000
+++ quickstart/manage.py	2013-12-04 13:19:54 +0000
@@ -25,10 +25,13 @@
 import quickstart
 from quickstart import (
     app,
-    charms,
     settings,
     utils,
 )
+from quickstart.models import (
+    charms,
+    envs,
+)
 
 
 version = quickstart.get_version()
@@ -144,7 +147,7 @@
         )
     # Validate the environment file.
     try:
-        env_type, admin_secret = utils.parse_env_file(env_file, env_name)
+        env_type, admin_secret = envs.parse_env_file(env_file, env_name)
     except ValueError as err:
         return parser.error(str(err))
     # Update the options namespace with the new values.
@@ -193,7 +196,7 @@
 
     Exit with an error if the provided arguments are not valid.
     """
-    default_env_name = utils.get_default_env_name()
+    default_env_name = envs.get_default_env_name()
     # Define the help message for the --environment option.
     env_help = 'The name of the Juju environment to use'
     # XXX 2013-12-02 makyo:

=== added directory 'quickstart/models'
=== added file 'quickstart/models/__init__.py'
--- quickstart/models/__init__.py	1970-01-01 00:00:00 +0000
+++ quickstart/models/__init__.py	2013-12-04 13:19:54 +0000
@@ -0,0 +1,17 @@
+# 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 models."""

=== renamed file 'quickstart/charms.py' => 'quickstart/models/charms.py'
=== added file 'quickstart/models/envs.py'
--- quickstart/models/envs.py	1970-01-01 00:00:00 +0000
+++ quickstart/models/envs.py	2013-12-04 13:19:54 +0000
@@ -0,0 +1,104 @@
+# 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 environments management."""
+
+import os
+import re
+
+import yaml
+
+from quickstart import utils
+
+
+# Compile the regular expression used to parse the "juju switch" output.
+_juju_switch_expression = re.compile(r'Current environment: "([\w-]+)"\n')
+
+
+def get_default_env_name():
+    """Return the current Juju environment name.
+
+    The environment name can be set either by
+        - setting the JUJU_ENV environment variable;
+        - using "juju switch my-env-name";
+        - setting the default environment in the environments.yaml file.
+    The former overrides the latter.
+
+    Return None if a default environment is not found.
+    """
+    env_name = os.getenv('JUJU_ENV', '').strip()
+    if env_name:
+        return env_name
+    # XXX 2013-10-16 frankban bug=1193244:
+        # Support the new behavior of juju-switch, currently under development:
+        # the command will just output the environment name, or exit with an
+        # error if no default environment is configured.
+    # The "juju switch" command parses ~/.juju/current-environment file. If the
+    # environment name is not found there, then it tries to retrieve the name
+    # from the "default" section of the ~/.juju/environments.yaml file.
+    retcode, output, _ = utils.call('juju', 'switch')
+    if retcode:
+        return None
+    match = _juju_switch_expression.match(output)
+    if match is not None:
+        return match.groups()[0]
+
+
+def parse_env_file(env_file, env_name):
+    """Parse the provided Juju environments.yaml file.
+
+    Return a tuple containing the provider type and the admin secret associated
+    with the given environment name.
+
+    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 environment named envname is not found;
+      - the environment does not include the "type" field;
+      - the environment does not include the "admin_secret" field.
+    """
+    # Load the Juju environments file.
+    try:
+        environments_file = open(env_file)
+    except IOError as err:
+        msg = 'unable to open environments file: {}'.format(err)
+        raise ValueError(msg)
+    # Parse the Juju environments file.
+    try:
+        environments = yaml.safe_load(environments_file)
+    except Exception as err:
+        msg = 'unable to parse environments file {}: {}'.format(env_file, err)
+        raise ValueError(msg)
+    # Retrieve the information about the current environment.
+    try:
+        environment = environments.get('environments', {}).get(env_name)
+    except AttributeError as err:
+        msg = 'invalid YAML contents in {}: {}'.format(env_file, environments)
+        raise ValueError(msg)
+    if environment is None:
+        msg = 'environment {} not found in {}'.format(env_name, env_file)
+        raise ValueError(msg)
+    # Retrieve the provider type and the admin secret.
+    env_type = environment.get('type')
+    if env_type is None:
+        msg = '{} provider type not found in {}'.format(env_name, env_file)
+        raise ValueError(msg)
+    admin_secret = environment.get('admin-secret')
+    if admin_secret is None:
+        msg = '{} admin secret not found in {}'.format(env_name, env_file)
+        raise ValueError(msg)
+    return env_type, admin_secret

=== added directory 'quickstart/tests/models'
=== added file 'quickstart/tests/models/__init__.py'
=== renamed file 'quickstart/tests/test_charms.py' => 'quickstart/tests/models/test_charms.py'
--- quickstart/tests/test_charms.py	2013-11-22 08:30:42 +0000
+++ quickstart/tests/models/test_charms.py	2013-12-04 13:19:54 +0000
@@ -18,7 +18,7 @@
 
 import unittest
 
-from quickstart import charms
+from quickstart.models import charms
 from quickstart.tests import helpers
 
 

=== added file 'quickstart/tests/models/test_envs.py'
--- quickstart/tests/models/test_envs.py	1970-01-01 00:00:00 +0000
+++ quickstart/tests/models/test_envs.py	2013-12-04 13:19:54 +0000
@@ -0,0 +1,136 @@
+# 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/>.
+
+"""Tests for the Juju Quickstart environments management."""
+
+import unittest
+
+import mock
+import yaml
+
+from quickstart.models import envs
+from quickstart.tests import helpers
+
+
+class TestGetDefaultEnvName(helpers.CallTestsMixin, unittest.TestCase):
+
+    def test_environment_variable(self):
+        # The environment name is successfully returned if JUJU_ENV is set.
+        with mock.patch('os.environ', {'JUJU_ENV': 'ec2'}):
+            env_name = envs.get_default_env_name()
+        self.assertEqual('ec2', env_name)
+
+    def test_empty_environment_variable(self):
+        # The environment name is not found if JUJU_ENV is empty.
+        with self.patch_call(1):
+            with mock.patch('os.environ', {'JUJU_ENV': ' '}):
+                env_name = envs.get_default_env_name()
+        self.assertIsNone(env_name)
+
+    def test_no_environment_variable(self):
+        # The environment name is not found if JUJU_ENV is not defined.
+        with self.patch_call(1):
+            with mock.patch('os.environ', {}):
+                env_name = envs.get_default_env_name()
+        self.assertIsNone(env_name)
+
+    def test_juju_switch(self):
+        # The environment name is successfully returned if previously set using
+        # the "juju switch" command.
+        output = 'Current environment: "hp"\n'
+        with self.patch_call(0, output=output) as mock_call:
+            with mock.patch('os.environ', {}):
+                env_name = envs.get_default_env_name()
+        self.assertEqual('hp', env_name)
+        mock_call.assert_called_once_with('juju', 'switch')
+
+    def test_juju_switch_failure(self):
+        # The environment name is not found if "juju switch" returns an error.
+        with self.patch_call(1) as mock_call:
+            with mock.patch('os.environ', {}):
+                env_name = envs.get_default_env_name()
+        self.assertIsNone(env_name)
+        mock_call.assert_called_once_with('juju', 'switch')
+
+    def test_juju_switch_nonsense(self):
+        # The environment name is not found if "juju switch" goes crazy.
+        with self.patch_call(0, output='Exterminate!') as mock_call:
+            with mock.patch('os.environ', {}):
+                env_name = envs.get_default_env_name()
+        self.assertIsNone(env_name)
+        mock_call.assert_called_once_with('juju', 'switch')
+
+
+class TestParseEnvFile(
+        helpers.EnvFileTestsMixin, helpers.ValueErrorTestsMixin,
+        unittest.TestCase):
+
+    def test_no_file(self):
+        # A ValueError is raised if the environments file is not found.
+        expected = (
+            'unable to open environments file: '
+            "[Errno 2] No such file or directory: '/no/such/file.yaml'"
+        )
+        with self.assert_value_error(expected):
+            envs.parse_env_file('/no/such/file.yaml', 'ec2')
+
+    def test_invalid_yaml(self):
+        # A ValueError is raised if the environments file is not a valid YAML.
+        env_file = self.make_env_file(':')
+        with self.assertRaises(ValueError) as context_manager:
+            envs.parse_env_file(env_file, 'ec2')
+        expected = 'unable to parse environments file {}'.format(env_file)
+        self.assertIn(expected, str(context_manager.exception))
+
+    def test_invalid_yaml_contents(self):
+        # A ValueError is raised if the environments file is not well formed.
+        env_file = self.make_env_file('a-string')
+        expected = 'invalid YAML contents in {}: a-string'.format(env_file)
+        with self.assert_value_error(expected):
+            envs.parse_env_file(env_file, 'ec2')
+
+    def test_no_env(self):
+        # A ValueError is raised if the environment is not found in the YAML.
+        contents = yaml.safe_dump({'environments': {'local': {}}})
+        env_file = self.make_env_file(contents)
+        expected = 'environment ec2 not found in {}'.format(env_file)
+        with self.assert_value_error(expected):
+            envs.parse_env_file(env_file, 'ec2')
+
+    def test_no_provider_type(self):
+        # A ValueError is raised if the provider type is not included in the
+        # environment info.
+        contents = yaml.safe_dump({'environments': {'aws': {}}})
+        env_file = self.make_env_file(contents)
+        expected = 'aws provider type not found in {}'.format(env_file)
+        with self.assert_value_error(expected):
+            envs.parse_env_file(env_file, 'aws')
+
+    def test_no_admin_secret(self):
+        # A ValueError is raised if the admin secret is not included in the
+        # environment info.
+        contents = yaml.safe_dump({'environments': {'aws': {'type': 'ec2'}}})
+        env_file = self.make_env_file(contents)
+        expected = 'aws admin secret not found in {}'.format(env_file)
+        with self.assert_value_error(expected):
+            envs.parse_env_file(env_file, 'aws')
+
+    def test_success(self):
+        # The environment provider type and admin secret are returned.
+        env_file = self.make_env_file()
+        env_type, admin_secret = envs.parse_env_file(env_file, 'aws')
+        self.assertEqual('ec2', env_type)
+        self.assertEqual('Secret!', admin_secret)

=== modified file 'quickstart/tests/test_manage.py'
--- quickstart/tests/test_manage.py	2013-12-02 21:36:04 +0000
+++ quickstart/tests/test_manage.py	2013-12-04 13:19:54 +0000
@@ -317,7 +317,7 @@
         and it is also possible to simulate an arbitrary environment name.
         """
         mock_get_default_env_name = mock.Mock(return_value=env_name)
-        path = 'quickstart.manage.utils.get_default_env_name'
+        path = 'quickstart.manage.envs.get_default_env_name'
         return mock.patch(path, mock_get_default_env_name)
 
     def call_setup(self, args, env_name='ec2', exit_called=True):
@@ -358,12 +358,11 @@
         output = stdout_write.call_args[0][0]
         self.assertIn('The name of the Juju environment to use (hp)\n', output)
 
-    def test_help_with_init_environment(self):
+    def test_init_environment(self):
         # The program calls ensure_environments if env_name is None.
         with mock.patch('quickstart.app.ensure_environments',
                         return_value='local') as ensure_environments:
-            self.call_setup(['/path/to/bundle.file'], env_name=None,
-                            exit_called=False)
+            self.call_setup([], env_name=None, exit_called=False)
         self.assertTrue(ensure_environments.called)
 
     def test_description(self):

=== modified file 'quickstart/tests/test_utils.py'
--- quickstart/tests/test_utils.py	2013-11-26 09:45:17 +0000
+++ quickstart/tests/test_utils.py	2013-12-04 13:19:54 +0000
@@ -235,55 +235,6 @@
             'unable to find the charm URL', str(context_manager.exception))
 
 
-class TestGetDefaultEnvName(helpers.CallTestsMixin, unittest.TestCase):
-
-    def test_environment_variable(self):
-        # The environment name is successfully returned if JUJU_ENV is set.
-        with mock.patch('os.environ', {'JUJU_ENV': 'ec2'}):
-            env_name = utils.get_default_env_name()
-        self.assertEqual('ec2', env_name)
-
-    def test_empty_environment_variable(self):
-        # The environment name is not found if JUJU_ENV is empty.
-        with self.patch_call(1):
-            with mock.patch('os.environ', {'JUJU_ENV': ' '}):
-                env_name = utils.get_default_env_name()
-        self.assertIsNone(env_name)
-
-    def test_no_environment_variable(self):
-        # The environment name is not found if JUJU_ENV is not defined.
-        with self.patch_call(1):
-            with mock.patch('os.environ', {}):
-                env_name = utils.get_default_env_name()
-        self.assertIsNone(env_name)
-
-    def test_juju_switch(self):
-        # The environment name is successfully returned if previously set using
-        # the "juju switch" command.
-        output = 'Current environment: "hp"\n'
-        with self.patch_call(0, output=output) as mock_call:
-            with mock.patch('os.environ', {}):
-                env_name = utils.get_default_env_name()
-        self.assertEqual('hp', env_name)
-        mock_call.assert_called_once_with('juju', 'switch')
-
-    def test_juju_switch_failure(self):
-        # The environment name is not found if "juju switch" returns an error.
-        with self.patch_call(1) as mock_call:
-            with mock.patch('os.environ', {}):
-                env_name = utils.get_default_env_name()
-        self.assertIsNone(env_name)
-        mock_call.assert_called_once_with('juju', 'switch')
-
-    def test_juju_switch_nonsense(self):
-        # The environment name is not found if "juju switch" goes crazy.
-        with self.patch_call(0, output='Exterminate!') as mock_call:
-            with mock.patch('os.environ', {}):
-                env_name = utils.get_default_env_name()
-        self.assertIsNone(env_name)
-        mock_call.assert_called_once_with('juju', 'switch')
-
-
 class TestGetServiceInfo(helpers.WatcherDataTestsMixin, unittest.TestCase):
 
     def test_service_and_unit(self):
@@ -487,68 +438,6 @@
         self.assert_bundle('mybundle', ['mysql', 'wordpress'], contents)
 
 
-class TestParseEnvFile(
-        helpers.EnvFileTestsMixin, helpers.ValueErrorTestsMixin,
-        unittest.TestCase):
-
-    def test_no_file(self):
-        # A ValueError is raised if the environments file is not found.
-        expected = (
-            'unable to open environments file: '
-            "[Errno 2] No such file or directory: '/no/such/file.yaml'"
-        )
-        with self.assert_value_error(expected):
-            utils.parse_env_file('/no/such/file.yaml', 'ec2')
-
-    def test_invalid_yaml(self):
-        # A ValueError is raised if the environments file is not a valid YAML.
-        env_file = self.make_env_file(':')
-        with self.assertRaises(ValueError) as context_manager:
-            utils.parse_env_file(env_file, 'ec2')
-        expected = 'unable to parse environments file {}'.format(env_file)
-        self.assertIn(expected, str(context_manager.exception))
-
-    def test_invalid_yaml_contents(self):
-        # A ValueError is raised if the environments file is not well formed.
-        env_file = self.make_env_file('a-string')
-        expected = 'invalid YAML contents in {}: a-string'.format(env_file)
-        with self.assert_value_error(expected):
-            utils.parse_env_file(env_file, 'ec2')
-
-    def test_no_env(self):
-        # A ValueError is raised if the environment is not found in the YAML.
-        contents = yaml.safe_dump({'environments': {'local': {}}})
-        env_file = self.make_env_file(contents)
-        expected = 'environment ec2 not found in {}'.format(env_file)
-        with self.assert_value_error(expected):
-            utils.parse_env_file(env_file, 'ec2')
-
-    def test_no_provider_type(self):
-        # A ValueError is raised if the provider type is not included in the
-        # environment info.
-        contents = yaml.safe_dump({'environments': {'aws': {}}})
-        env_file = self.make_env_file(contents)
-        expected = 'aws provider type not found in {}'.format(env_file)
-        with self.assert_value_error(expected):
-            utils.parse_env_file(env_file, 'aws')
-
-    def test_no_admin_secret(self):
-        # A ValueError is raised if the admin secret is not included in the
-        # environment info.
-        contents = yaml.safe_dump({'environments': {'aws': {'type': 'ec2'}}})
-        env_file = self.make_env_file(contents)
-        expected = 'aws admin secret not found in {}'.format(env_file)
-        with self.assert_value_error(expected):
-            utils.parse_env_file(env_file, 'aws')
-
-    def test_success(self):
-        # The environment provider type and admin secret are returned.
-        env_file = self.make_env_file()
-        env_type, admin_secret = utils.parse_env_file(env_file, 'aws')
-        self.assertEqual('ec2', env_type)
-        self.assertEqual('Secret!', admin_secret)
-
-
 class TestParseStatusOutput(helpers.ValueErrorTestsMixin, unittest.TestCase):
 
     def test_invalid_yaml(self):

=== modified file 'quickstart/utils.py'
--- quickstart/utils.py	2013-11-25 18:07:47 +0000
+++ quickstart/utils.py	2013-12-04 13:19:54 +0000
@@ -21,22 +21,15 @@
 import httplib
 import json
 import logging
-import os
 import pipes
-import re
 import socket
 import subprocess
 import urllib2
 
 import yaml
 
-from quickstart import (
-    charms,
-    settings
-)
-
-# Compile the regular expression used to parse the "juju switch" output.
-_juju_switch_expression = re.compile(r'Current environment: "([\w-]+)"\n')
+from quickstart import settings
+from quickstart.models import charms
 
 
 def add_apt_repository(repository):
@@ -134,35 +127,6 @@
     return charm_url
 
 
-def get_default_env_name():
-    """Return the current Juju environment name.
-
-    The environment name can be set either by
-        - setting the JUJU_ENV environment variable;
-        - using "juju switch my-env-name";
-        - setting the default environment in the environments.yaml file.
-    The former overrides the latter.
-
-    Return None if a default environment is not found.
-    """
-    env_name = os.getenv('JUJU_ENV', '').strip()
-    if env_name:
-        return env_name
-    # XXX 2013-10-16 frankban bug=1193244:
-        # Support the new behavior of juju-switch, currently under development:
-        # the command will just output the environment name, or exit with an
-        # error if no default environment is configured.
-    # The "juju switch" command parses ~/.juju/current-environment file. If the
-    # environment name is not found there, then it tries to retrieve the name
-    # from the "default" section of the ~/.juju/environments.yaml file.
-    retcode, output, _ = call('juju', 'switch')
-    if retcode:
-        return None
-    match = _juju_switch_expression.match(output)
-    if match is not None:
-        return match.groups()[0]
-
-
 def get_service_info(status, service_name):
     """Retrieve information on the given service and on its first alive unit.
 
@@ -258,53 +222,6 @@
     return bundle_name, bundle_services
 
 
-def parse_env_file(env_file, env_name):
-    """Parse the provided Juju environments.yaml file.
-
-    Return a tuple containing the provider type and the admin secret associated
-    with the given environment name.
-
-    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 environment named envname is not found;
-      - the environment does not include the "type" field;
-      - the environment does not include the "admin_secret" field.
-    """
-    # Load the Juju environments file.
-    try:
-        environments_file = open(env_file)
-    except IOError as err:
-        msg = 'unable to open environments file: {}'.format(err)
-        raise ValueError(msg)
-    # Parse the Juju environments file.
-    try:
-        environments = yaml.safe_load(environments_file)
-    except Exception as err:
-        msg = 'unable to parse environments file {}: {}'.format(env_file, err)
-        raise ValueError(msg)
-    # Retrieve the information about the current environment.
-    try:
-        environment = environments.get('environments', {}).get(env_name)
-    except AttributeError as err:
-        msg = 'invalid YAML contents in {}: {}'.format(env_file, environments)
-        raise ValueError(msg)
-    if environment is None:
-        msg = 'environment {} not found in {}'.format(env_name, env_file)
-        raise ValueError(msg)
-    # Retrieve the provider type and the admin secret.
-    env_type = environment.get('type')
-    if env_type is None:
-        msg = '{} provider type not found in {}'.format(env_name, env_file)
-        raise ValueError(msg)
-    admin_secret = environment.get('admin-secret')
-    if admin_secret is None:
-        msg = '{} admin secret not found in {}'.format(env_name, env_file)
-        raise ValueError(msg)
-    return env_type, admin_secret
-
-
 def parse_status_output(output):
     """Parse the output of juju status.