Code review comment for lp:~frankban/juju-quickstart/ppa-flag

Reviewers: mp+214536_code.launchpad.net,

Message:
Please take a look.

Description:
Support the --ppa flag for distro packaging.

Add a packaging module suitable for being
easily modified when packaging quickstart for
distro repositories.

The new --ppa flag can be used to switch to ppa
when quickstart is installed from the default
repositories.

Tests: `make check`.

QA: there is no easy way to QA this.
It would require removing juju packages,
running quickstart with and without
the --distro-only and --ppa flags,
switching packaging.py to "distro"
and trying again.
I'll do that anyway as part of the next
release QA, so feel free to avoid QAing now.

https://code.launchpad.net/~frankban/juju-quickstart/ppa-flag/+merge/214536

(do not edit description out of merge proposal)

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

Affected files (+87, -1 lines):
   A [revision details]
   M quickstart/manage.py
   A quickstart/packaging.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-03-27 10:38:01 +0000
+++ quickstart/manage.py 2014-04-07 10:11:34 +0000
@@ -32,6 +32,7 @@
  import quickstart
  from quickstart import (
      app,
+ packaging,
      settings,
      utils,
  )
@@ -53,6 +54,29 @@
          parser.exit()

+def _get_packaging_info(juju_source):
+ """Return packaging info based on the given juju source.
+
+ The juju_source argument can be either "ppa" or "distro".
+
+ Packaging info is a tuple containing:
+ - distro_only: whether to install juju-core packages from the
distro
+ repositories or the external PPA;
+ - distro_only_help: the help text for the --distro-only flag;
+ - ppa_help: the help text for the --ppa flag.
+ """
+ distro_only_help = ('Do not use external sources when installing and '
+ 'setting up Juju')
+ ppa_help = 'Use external sources when installing and setting up Juju'
+ if juju_source == 'distro':
+ distro_only = True
+ distro_only_help += '\n(enabled by default, use --ppa to disable)'
+ else:
+ distro_only = False
+ ppa_help += '\n(enabled by default, use --distro-only to disable)'
+ return distro_only, distro_only_help, ppa_help
+
+
  def _validate_bundle(options, parser):
      """Validate and process the bundle options.

@@ -317,6 +341,8 @@
      Exit with an error if the provided arguments are not valid.
      """
      default_env_name = envs.get_default_env_name()
+ default_distro_only, distro_only_help, ppa_help = _get_packaging_info(
+ packaging.JUJU_SOURCE)
      # Define the help message for the --environment option.
      env_help = 'The name of the Juju environment to use'
      if default_env_name is not None:
@@ -377,7 +403,10 @@
          help='Avoid opening the browser to the GUI at the end of
the\nprocess')
      parser.add_argument(
          '--distro-only', action='store_true', dest='distro_only',
- help='Do not use external sources when installing and setting up
Juju')
+ default=default_distro_only, help=distro_only_help)
+ parser.add_argument(
+ '--ppa', action='store_false', dest='distro_only',
+ default=not default_distro_only, help=ppa_help)
      parser.add_argument(
          '--version', action='version', version='%(prog)s
{}'.format(version))
      parser.add_argument(

Index: quickstart/packaging.py
=== added file 'quickstart/packaging.py'
--- quickstart/packaging.py 1970-01-01 00:00:00 +0000
+++ quickstart/packaging.py 2014-04-07 10:10:13 +0000
@@ -0,0 +1,31 @@
+# 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) 2014 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 packaging configuration.
+
+This module is parsed and modified in the process of packaging quickstart
+for Ubuntu distributions.
+
+DO NOT MODIFY this file without informing server/distro developers.
+"""
+
+# The source from where to install juju-core packages.
+# Possible values are:
+# - ppa: the Juju stable packages PPA. This value is usually set in the
code
+# base and PyPI releases;
+# - distro: the distribution repository. This value is usually set in
the deb
+# releases included in the Ubuntu repositories.
+JUJU_SOURCE = 'ppa'

Index: quickstart/tests/test_manage.py
=== modified file 'quickstart/tests/test_manage.py'
--- quickstart/tests/test_manage.py 2014-03-14 16:50:22 +0000
+++ quickstart/tests/test_manage.py 2014-04-07 10:29:29 +0000
@@ -58,6 +58,30 @@
          mock_exit.assert_called_once_with(0)

+class TestGetPackagingInfo(unittest.TestCase):
+
+ distro_only_disable = '(enabled by default, use --ppa to disable)'
+ ppa_disable = '(enabled by default, use --distro-only to disable)'
+
+ def test_ppa_source(self):
+ # The returned distro_only flag is set to False and the help texts
are
+ # formatted accordingly when the passed Juju source is "ppa".
+ distro_only, distro_only_help, ppa_help =
manage._get_packaging_info(
+ 'ppa')
+ self.assertFalse(distro_only)
+ self.assertNotIn(self.distro_only_disable, distro_only_help)
+ self.assertIn(self.ppa_disable, ppa_help)
+
+ def test_distro_source(self):
+ # The returned distro_only flag is set to True and the help texts
are
+ # formatted accordingly when the passed Juju source is "distro".
+ distro_only, distro_only_help, ppa_help =
manage._get_packaging_info(
+ 'distro')
+ self.assertTrue(distro_only)
+ self.assertIn(self.distro_only_disable, distro_only_help)
+ self.assertNotIn(self.ppa_disable, ppa_help)
+
+
  class TestValidateBundle(
          helpers.BundleFileTestsMixin, helpers.UrlReadTestsMixin,
          unittest.TestCase):