Merge lp:~frankban/juju-quickstart/improve-venv into lp:juju-quickstart

[Francesco Banconi]

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

[Francesco Banconi (frankban)]

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

[Gary Poster (gary)]

Nice. LGTM with trivial comments.

https://codereview.appspot.com/26540043/diff/1/HACKING.rst
File HACKING.rst (right):

https://codereview.appspot.com/26540043/diff/1/HACKING.rst#newcode97
HACKING.rst:97: Also ensure, before updating the application
dependencies, that those packages
I suggest that you make this a second paragraph, since it is a separate
topic.

https://codereview.appspot.com/26540043/diff/1/MANIFEST.in
File MANIFEST.in (right):

https://codereview.appspot.com/26540043/diff/1/MANIFEST.in#newcode23
MANIFEST.in:23: # distribution and therefore the Debian package builds.
Do not do that.
Thank you for this comment!

https://codereview.appspot.com/26540043/diff/1/Makefile
File Makefile (right):

https://codereview.appspot.com/26540043/diff/1/Makefile#newcode27
Makefile:27: (rm -f $(VENV_ACTIVATE); exit 1)
same comment as with gui charm: instead of rm'ing, let's touch
test-requirements.pip.

https://codereview.appspot.com/26540043/

[lp:~frankban/juju-quickstart/improve-venv]

18. By Francesco Banconi on 2013-11-14

Changes as per review.

[Francesco Banconi (frankban)]

*** Submitted:

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.

R=gary.poster
CC=
https://codereview.appspot.com/26540043

https://codereview.appspot.com/26540043/diff/1/HACKING.rst
File HACKING.rst (right):

https://codereview.appspot.com/26540043/diff/1/HACKING.rst#newcode97
HACKING.rst:97: Also ensure, before updating the application
dependencies, that those packages
On 2013/11/14 12:04:01, gary.poster wrote:
> I suggest that you make this a second paragraph, since it is a
separate topic.

Done.

https://codereview.appspot.com/26540043/diff/1/Makefile
File Makefile (right):

https://codereview.appspot.com/26540043/diff/1/Makefile#newcode27
Makefile:27: (rm -f $(VENV_ACTIVATE); exit 1)
On 2013/11/14 12:04:01, gary.poster wrote:
> same comment as with gui charm: instead of rm'ing, let's touch
> test-requirements.pip.

Done.

https://codereview.appspot.com/26540043/

=== modified file 'HACKING.rst'
--- HACKING.rst	2013-11-12 15:34:15 +0000
+++ HACKING.rst	2013-11-14 12:21:11 +0000
@@ -83,3 +83,19 @@
 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>.

=== added file 'MANIFEST.in'
--- MANIFEST.in	1970-01-01 00:00:00 +0000
+++ MANIFEST.in	2013-11-14 12:21:11 +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

=== modified file 'Makefile'
--- Makefile	2013-10-30 10:16:36 +0000
+++ Makefile	2013-11-14 12:21:11 +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 || \
+		(touch test-requirements.pip; exit 1)
 	@touch $(VENV_ACTIVATE)
 
 all: setup

=== modified file 'quickstart/settings.py'
--- quickstart/settings.py	2013-11-13 12:23:36 +0000
+++ quickstart/settings.py	2013-11-14 12:21:11 +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'

=== added file 'requirements.pip'
--- requirements.pip	1970-01-01 00:00:00 +0000
+++ requirements.pip	2013-11-14 12:21:11 +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

=== modified file 'setup.py'
--- setup.py	2013-11-06 15:21:19 +0000
+++ setup.py	2013-11-14 12:21:11 +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',

=== modified file 'test-requirements.pip'
--- test-requirements.pip	2013-10-30 10:16:36 +0000
+++ test-requirements.pip	2013-11-14 12:21:11 +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