Code review comment for lp://staging/~bac/juju-quickstart/local-error-osx

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.
          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.