Code review comment for lp:~frankban/juju-quickstart/improve-venv

Reviewers: mp+195200_code.launchpad.net,

Message:
Please take a look.

Description:
Update the quickstart test venv set up process.

Remove the activate file if the requirements
installation process fails.

Also updated the default GUI charm URL, and
the documentation.

https://code.launchpad.net/~frankban/juju-quickstart/improve-venv/+merge/195200

(do not edit description out of merge proposal)

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

Affected files (+76, -11 lines):
   M HACKING.rst
   A MANIFEST.in
   M Makefile
   A [revision details]
   M quickstart/settings.py
   A requirements.pip
   M setup.py
   M test-requirements.pip

Index: HACKING.rst
=== modified file 'HACKING.rst'
--- HACKING.rst 2013-11-12 15:34:15 +0000
+++ HACKING.rst 2013-11-14 10:50:46 +0000
@@ -83,3 +83,18 @@
  Packages depend on `python-jujuclient` and `python-websocket-client` to be
  available. They are available in saucy, and they are also stored in our
PPA in
  order to support previous Ubuntu releases.
+
+Updating application and test dependencies
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Test dependencies are listed in the ``test-requirements.pip`` file in the
+branch root, application ones in the ``requirements.pip`` file. The former
+includes the latter, so any updates to the application requirements will
also
+update the test dependencies and therefore the testing virtual environment.
+Note that, since the source requirements are dynamically generated parsing
+``requirements.pip``, that file must only include PACKAGE==VERSION
formatted
+dependencies, and not other pip specific requirement specifications.
+Also ensure, before updating the application dependencies, that those
packages
+are available in the main Ubuntu repositories for the series we support
(from
+precise to saucy), or in the Juju Quickstart Beta PPA: see
+<https://launchpad.net/~juju-gui/+archive/quickstart-beta/+packages>.

Index: MANIFEST.in
=== added file 'MANIFEST.in'
--- MANIFEST.in 1970-01-01 00:00:00 +0000
+++ MANIFEST.in 2013-11-14 10:37:29 +0000
@@ -0,0 +1,24 @@
+# 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) 2012-2013 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/>.
+
+include COPYING
+include HACKING.rst
+include MANIFEST.in
+include README.rst
+# Note: since the source requirements are dynamically generated parsing the
+# requirements.pip file, removing it from this list will break the source
+# distribution and therefore the Debian package builds. Do not do that.
+include requirements.pip

Index: Makefile
=== modified file 'Makefile'
--- Makefile 2013-10-30 10:16:36 +0000
+++ Makefile 2013-11-14 10:32:15 +0000
@@ -21,9 +21,10 @@
  VENV_ACTIVATE = $(VENV)/bin/activate

-$(VENV_ACTIVATE): test-requirements.pip
+$(VENV_ACTIVATE): test-requirements.pip requirements.pip
   virtualenv --distribute -p $(PYTHON) $(VENV)
- $(VENV)/bin/pip install --use-mirrors -r test-requirements.pip
+ $(VENV)/bin/pip install --use-mirrors -r test-requirements.pip || \
+ (rm -f $(VENV_ACTIVATE); exit 1)
   @touch $(VENV_ACTIVATE)

  all: setup

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: requirements.pip
=== added file 'requirements.pip'
--- requirements.pip 1970-01-01 00:00:00 +0000
+++ requirements.pip 2013-11-14 10:37:29 +0000
@@ -0,0 +1,25 @@
+# Juju Quickstart application requirements.
+
+# 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) 2013 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/>.
+
+# Note: since the source requirements are dynamically generated parsing the
+# requirements.pip file, the list below must only include PACKAGE==VERSION
+# formatted dependencies. Do not include other pip specific requirement
+# specifications (e.g. -e ... or -r ...).
+
+jujuclient==0.11
+PyYAML==3.10

Index: setup.py
=== modified file 'setup.py'
--- setup.py 2013-11-06 15:21:19 +0000
+++ setup.py 2013-11-14 10:37:29 +0000
@@ -26,14 +26,14 @@

  ROOT = os.path.abspath(os.path.dirname(__file__))
  PROJECT_NAME = 'quickstart'
-REQUIREMENTS = (
- 'jujuclient==0.11',
- 'PyYAML==3.10',
-)

  project = __import__(PROJECT_NAME)
  description_path = os.path.join(ROOT, 'README.rst')

+requirements_path = os.path.join(ROOT, 'requirements.pip')
+requirements = [i.strip() for i in open(requirements_path).readlines()]
+install_requires = [i for i in requirements if i and not i.startswith('#')]
+
  os.chdir(ROOT)

  data_files = []
@@ -49,7 +49,6 @@
                  dirpath[len(PROJECT_NAME) + 1:], f))

-os.chdir(ROOT)
  setup(
      name='juju-quickstart',
      version=project.get_version(),
@@ -62,7 +61,7 @@
      packages=find_packages(),
      package_data={PROJECT_NAME: data_files},
      scripts=['juju-quickstart'],
- install_requires=REQUIREMENTS,
+ install_requires=install_requires,
      zip_safe=False,
      classifiers=[
          'Development Status :: 3 - Alpha',

Index: quickstart/settings.py
=== modified file 'quickstart/settings.py'
--- quickstart/settings.py 2013-11-13 12:23:36 +0000
+++ quickstart/settings.py 2013-11-14 10:52:31 +0000
@@ -24,7 +24,7 @@

  # The default Juju GUI charm URL to use when it is not possible to
retrieve it
  # from the charmworld API, e.g. due to temporary connection/charmworld
errors.
-DEFAULT_CHARM_URL = 'cs:precise/juju-gui-79'
+DEFAULT_CHARM_URL = 'cs:precise/juju-gui-80'

  # The quickstart app short description.
  DESCRIPTION = 'set up a Juju environment (including the GUI) in very few
steps'

Index: test-requirements.pip
=== modified file 'test-requirements.pip'
--- test-requirements.pip 2013-10-30 10:16:36 +0000
+++ test-requirements.pip 2013-11-14 10:32:15 +0000
@@ -18,7 +18,6 @@

  coverage==3.7
  flake8==2.0
-jujuclient==0.11
  mock==1.0.1
  nose==1.3.0
-PyYAML==3.10
+-r requirements.pip