Merge into trunk : ppa-flag : Code : juju-quickstart

Status:	Merged
Merged at revision:	63
Proposed branch:	lp://qastaging/~frankban/juju-quickstart/ppa-flag
Merge into:	lp://qastaging/juju-quickstart
Diff against target:	147 lines (+87/-2) 4 files modified quickstart/__init__.py (+1/-1) quickstart/manage.py (+31/-1) quickstart/packaging.py (+31/-0) quickstart/tests/test_manage.py (+24/-0)
To merge this branch:	bzr merge lp://qastaging/~frankban/juju-quickstart/ppa-flag
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Juju GUI Hackers		2014-04-07	Pending
Review via email: mp+214536@code.qastaging.launchpad.net

Description of the change

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/

Revision history for this message

Francesco Banconi (frankban) wrote on 2014-04-07:

#

Download full text (6.8 KiB)

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

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:  
francesco.banconi@canonical.com-20140407132546-hxx0jv9vnnwjw8ap
+New revision:  
francesco.banconi@canonical.com-20140407132734-276gr6o8uf6e7sew

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):

Revision history for this message

Brad Crittenden (bac) wrote on 2014-04-07:

#

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://qastaging/~frankban/juju-quickstart/ppa-flag updated on 2014-04-07

74. By Francesco Banconi on 2014-04-07: Changes as per review.

Revision history for this message

Francesco Banconi (frankban) wrote on 2014-04-07:

#

*** 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/

Revision history for this message

Francesco Banconi (frankban) wrote on 2014-04-07:

#

Thank you Brad!

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

juju-quickstart

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

Commit message

Description of the change

Preview Diff

Subscribers