Merge lp:~frankban/juju-quickstart/ppa-flag into lp:juju-quickstart

[Francesco Banconi]

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://codereview.appspot.com/84520047/

[Francesco Banconi (frankban)]

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

[Brad Crittenden (bac)]

LGTM

https://codereview.appspot.com/84520047/diff/1/quickstart/manage.py
File quickstart/manage.py (right):

https://codereview.appspot.com/84520047/diff/1/quickstart/manage.py#newcode76
quickstart/manage.py:76: ppa_help += '\n(enabled by default, use
--distro-only to disable)'
DRY: pull out this message into a template that gets populated
appropriately? +0

https://codereview.appspot.com/84520047/

[lp:~frankban/juju-quickstart/ppa-flag]

74. By Francesco Banconi on 2014-04-07

Changes as per review.

[Francesco Banconi (frankban)]

*** Submitted:

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.

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

https://codereview.appspot.com/84520047/diff/1/quickstart/manage.py
File quickstart/manage.py (right):

https://codereview.appspot.com/84520047/diff/1/quickstart/manage.py#newcode76
quickstart/manage.py:76: ppa_help += '\n(enabled by default, use
--distro-only to disable)'
On 2014/04/07 14:46:33, bac wrote:
> DRY: pull out this message into a template that gets populated
appropriately?
> +0

Done.

https://codereview.appspot.com/84520047/

[Francesco Banconi (frankban)]

Thank you Brad!

https://codereview.appspot.com/84520047/

=== modified file 'quickstart/__init__.py'
--- quickstart/__init__.py	2014-03-27 10:44:34 +0000
+++ quickstart/__init__.py	2014-04-07 15:53:06 +0000
@@ -45,7 +45,7 @@
 Once Juju has been installed, the command can also be run as a juju plugin,
 without the hyphen ("juju quickstart").
 """
-VERSION = (1, 3, 0)
+VERSION = (1, 3, 1)
 
 
 def get_version():

=== modified file 'quickstart/manage.py'
--- quickstart/manage.py	2014-03-27 10:38:01 +0000
+++ quickstart/manage.py	2014-04-07 15:53:06 +0000
@@ -32,6 +32,7 @@
 import quickstart
 from quickstart import (
     app,
+    packaging,
     settings,
     utils,
 )
@@ -53,6 +54,30 @@
         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'
+    disable_help = '\n(enabled by default, use {} to disable)'
+    if juju_source == 'distro':
+        distro_only = True
+        distro_only_help += disable_help.format('--ppa')
+    else:
+        distro_only = False
+        ppa_help += disable_help.format('--distro-only')
+    return distro_only, distro_only_help, ppa_help
+
+
 def _validate_bundle(options, parser):
     """Validate and process the bundle options.
 
@@ -317,6 +342,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 +404,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(

=== added file 'quickstart/packaging.py'
--- quickstart/packaging.py	1970-01-01 00:00:00 +0000
+++ quickstart/packaging.py	2014-04-07 15:53:06 +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'

=== 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 15:53:06 +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):