Merge lp:~bac/juju-quickstart/local-error-osx into lp:juju-quickstart

[Brad Crittenden]

Error if local env requested but not supported.

If '-e some-local-environment' is used on a host that does not support LXC,
then bootstrap will be called and an obscure 'apt-config' error is thrown.
So, let's not do that. Instead, detect the error early and return an argparse
error that makes sense.

https://codereview.appspot.com/104940045/

[Brad Crittenden (bac)]

Reviewers: mp+222562_code.launchpad.net,

Message:
Please take a look.

Description:
Error if local env requested but not supported.

If '-e some-local-environment' is used on a host that does not support
LXC,
then bootstrap will be called and an obscure 'apt-config' error is
thrown.
So, let's not do that. Instead, detect the error early and return an
argparse
error that makes sense.

https://code.launchpad.net/~bac/juju-quickstart/local-error-osx/+merge/222562

(do not edit description out of merge proposal)

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

Affected files (+31, -3 lines):
   A [revision details]
   M quickstart/manage.py
   M quickstart/tests/test_manage.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/manage.py
=== modified file 'quickstart/manage.py'
--- quickstart/manage.py 2014-06-02 12:56:24 +0000
+++ quickstart/manage.py 2014-06-09 19:21:43 +0000
@@ -285,11 +285,17 @@
          # This is a non-interactive session and we need to validate the
          # selected environment before proceeding.
          env_data = _retrieve_env_data(parser, env_type_db, env_db,
env_name)
+ # Check for local support, if requested.
+ options.env_type = env_data['type']
+ no_local_support = not
platform_support.supports_local(options.platform)
+ if options.env_type == 'local' and no_local_support:
+ return parser.error(
+ 'This host platform does not support local environments.'
+ )
      # Update the options namespace with the new values.
      options.admin_secret = env_data.get('admin-secret')
      options.env_file = env_file
      options.env_name = env_data['name']
- options.env_type = env_data['type']
      options.default_series = env_data.get('default-series')
      options.interactive = interactive

Index: quickstart/tests/test_manage.py
=== modified file 'quickstart/tests/test_manage.py'
--- quickstart/tests/test_manage.py 2014-06-02 12:56:24 +0000
+++ quickstart/tests/test_manage.py 2014-06-09 19:21:43 +0000
@@ -462,12 +462,14 @@
      def setUp(self):
          self.parser = mock.Mock()

- def make_options(self, env_file, env_name=None, interactive=False):
+ def make_options(self, env_file, env_name=None, interactive=False,
+ platform=settings.LINUX_APT):
          """Return a mock options object which includes the passed
arguments."""
          return mock.Mock(
              env_file=env_file,
              env_name=env_name,
              interactive=interactive,
+ platform=platform,
          )

      def patch_interactive_mode(self, return_value):
@@ -516,7 +518,7 @@
          message = self.parser.error.call_args[0][0]
          self.assertIn('unable to find an environment name to use', message)

- def test_local_provider(self):
+ def test_local_provider_linux(self):
          # Local environments are correctly handled.
     ...

[Brad Crittenden (bac)]

LGTM self-review.

https://codereview.appspot.com/104940045/

[Brad Crittenden (bac)]

*** Submitted:

Error if local env requested but not supported.

If '-e some-local-environment' is used on a host that does not support
LXC,
then bootstrap will be called and an obscure 'apt-config' error is
thrown.
So, let's not do that. Instead, detect the error early and return an
argparse
error that makes sense.

R=
CC=
https://codereview.appspot.com/104940045

https://codereview.appspot.com/104940045/

=== modified file 'quickstart/manage.py'
--- quickstart/manage.py	2014-06-02 12:56:24 +0000
+++ quickstart/manage.py	2014-06-09 21:03:17 +0000
@@ -285,11 +285,17 @@
         # This is a non-interactive session and we need to validate the
         # selected environment before proceeding.
         env_data = _retrieve_env_data(parser, env_type_db, env_db, env_name)
+    # Check for local support, if requested.
+    options.env_type = env_data['type']
+    no_local_support = not platform_support.supports_local(options.platform)
+    if options.env_type == 'local' and no_local_support:
+        return parser.error(
+            'This host platform does not support local environments.'
+        )
     # Update the options namespace with the new values.
     options.admin_secret = env_data.get('admin-secret')
     options.env_file = env_file
     options.env_name = env_data['name']
-    options.env_type = env_data['type']
     options.default_series = env_data.get('default-series')
     options.interactive = interactive
 

=== modified file 'quickstart/tests/test_manage.py'
--- quickstart/tests/test_manage.py	2014-06-02 12:56:24 +0000
+++ quickstart/tests/test_manage.py	2014-06-09 21:03:17 +0000
@@ -462,12 +462,14 @@
     def setUp(self):
         self.parser = mock.Mock()
 
-    def make_options(self, env_file, env_name=None, interactive=False):
+    def make_options(self, env_file, env_name=None, interactive=False,
+                     platform=settings.LINUX_APT):
         """Return a mock options object which includes the passed arguments."""
         return mock.Mock(
             env_file=env_file,
             env_name=env_name,
             interactive=interactive,
+            platform=platform,
         )
 
     def patch_interactive_mode(self, return_value):
@@ -516,7 +518,7 @@
         message = self.parser.error.call_args[0][0]
         self.assertIn('unable to find an environment name to use', message)
 
-    def test_local_provider(self):
+    def test_local_provider_linux(self):
         # Local environments are correctly handled.
         contents = yaml.safe_dump({
             'environments': {
@@ -534,6 +536,24 @@
         self.assertIsNone(options.default_series)
         self.assertFalse(options.interactive)
 
+    def test_local_provider_not_linux(self):
+        # Local environments are correctly handled.
+        contents = yaml.safe_dump({
+            'environments': {
+                'lxc': {'admin-secret': 'Secret!', 'type': 'local'},
+            },
+        })
+        env_file = self.make_env_file(contents)
+        options = self.make_options(
+            env_file, env_name='lxc', interactive=False,
+            platform=settings.OSX)
+        manage._setup_env(options, self.parser)
+        self.assertTrue(self.parser.error.called)
+        message = self.parser.error.call_args[0][0]
+        self.assertEqual(
+            'This host platform does not support local environments.',
+            message)
+
     def test_interactive_mode(self):
         # The interactive mode is started properly if the corresponding option
         # flag is set.