Merge lp:~codersquid/charms/trusty/python-django/pip-extra-args into lp:charms/python-django

[Sheila Miguez]

This change adds pip_extra_args functionality. hooks.py now uses the pip_extra_args config option. This allows people to specify useful things like --no-index and --find-links.

Based on the ability of a user to set pip_extra_args:.

* parameters that were getting passed to pip were removed, i.e. --upgrade, --use-mirrors, -b, --src.
* since --upgrade was a default in hooks.py, I set the config default to use --upgrade.

This change adds pip_extra_args functionality. hooks.py now uses the pip_extra_args config option. This allows people to specify useful things like --no-index and --find-links.

Based on the ability of a user to set pip_extra_args:.

* parameters that were getting passed to pip were removed, i.e. --upgrade, --use-mirrors, -b, --src.
* since --upgrade was a default in hooks.py, I set the config default to use --upgrade.

[Review Queue (review-queue)]

This items has failed automated testing! Results available here http://reports.vapour.ws/charm-tests/charm-bundle-test-10877-results

review: Needs Fixing (automated testing)

[Cory Johns (johnsca)]

Sheila,

Thank you for this contribution to the python-django charm. The test failure appears to be valid, however it seems to be failing due to an existing value in tests/config/django.yaml that was previously being ignored and this change activates.

I think that if you change the "-U -i http://10.0.3.1:3141/root/pypi/" option in the test bundle to something that won't interfere with the install process, you should be able to resolve the test.

One other thing to note, which is also not directly changed in this MP but should probably be addressed, is that the tests are hard-coded in django.yaml to use the precise versions of all of the charms and may not properly test this charms changes. It should be easy enough to change the secondary charms to point to the trusty versions and to remove the branch from the python-django charm entry to have it use the code from this charm.

Thanks again for this contribution, and I look forward to seeing it merged.

review: Needs Fixing

[Cory Johns (johnsca)]

For reference, to reproduce this, I ran the tests using bundletester (which is how the automated test runner runs them): https://pypi.python.org/pypi/bundletester/0.4.4

[Sheila Miguez (codersquid)]

Hi Cory, I'm finally getting around to working on this again. I want to make sure I'm invoking bundletester correctly before fixing my changes. It currently fails on trunk. I've branched trunk, and tried this from the top level directory of my branch.

$ bundletester -l DEBUG -o result --testdir tests
DEBUG:bundletester.utils:Updating JUJU_ENV: "" -> "local"
DEBUG:root:Bootstrap environment: local
DEBUG:deployer.env:Connecting to environment...
DEBUG:deployer.env:Connected to environment
DEBUG:deployer.env: Terminating machines
DEBUG:root:Waiting for services to be removed...
DEBUG:runner:call ['/tmp/bundletester-HIXG6W/tests/01-dj13']
DEBUG:runner:2015-03-08 15:20:32,430 INFO (cwd=/tmp/tmpbe8l4e3q) RUN: ['juju-deployer', '-vdWL', '-c', '/tmp/bundletester-HIXG6W/tests/config/django.yaml', 'django13', '--timeout', '2000']
DEBUG:runner:2015-03-08 15:20:32 [DEBUG] deployer.cli: Using runtime GoEnvironment on local
DEBUG:runner:2015-03-08 15:20:32 [INFO] deployer.cli: Starting deployment of django13
DEBUG:runner:2015-03-08 15:20:32 [DEBUG] deployer.import: Getting charms...
DEBUG:runner:2015-03-08 15:20:32 [DEBUG] deployer.charm: Cache dir /home/ubuntu/.juju/.deployer-store-cache/cs_precise_gunicorn
DEBUG:runner:2015-03-08 15:20:32 [DEBUG] deployer.deploy: Resolving configuration
DEBUG:runner:2015-03-08 15:20:32 [ERROR] deployer.deploy: Invalid config charm python-django pip_extra_args=-U -i http://10.0.3.1:3141/root/pypi/
DEBUG:runner:2015-03-08 15:20:32 [ERROR] deployer.deploy: Invalid config charm python-django django_debug=True
DEBUG:runner:2015-03-08 15:20:32 [ERROR] deployer.deploy: Invalid config charm python-django django_version=django>=1.3,<1.4
DEBUG:runner:2015-03-08 15:20:32 [INFO] deployer.cli: Deployment stopped. run time: 0.22
DEBUG:runner:ERROR
DEBUG:runner:
DEBUG:runner:======================================================================
DEBUG:runner:ERROR: setUpModule (__main__)
DEBUG:runner:----------------------------------------------------------------------
DEBUG:runner:Traceback (most recent call last):
DEBUG:runner: File "/tmp/bundletester-HIXG6W/tests/01-dj13", line 29, in setUpModule
DEBUG:runner: timeout=2000)
DEBUG:runner: File "/tmp/bundletester-HIXG6W/tests/jujulib/deployer.py", line 42, in deploy
DEBUG:runner: subprocess.check_call(args, cwd=deployer_dir)
DEBUG:runner: File "/usr/lib/python3.4/subprocess.py", line 557, in check_call
DEBUG:runner: raise CalledProcessError(retcode, cmd)
DEBUG:runner:subprocess.CalledProcessError: Command '['juju-deployer', '-vdWL', '-c', '/tmp/bundletester-HIXG6W/tests/config/django.yaml', 'django13', '--timeout', '2000']' returned non-zero exit status 1
DEBUG:runner:
DEBUG:runner:----------------------------------------------------------------------
DEBUG:runner:Ran 0 tests in 0.396s
DEBUG:runner:
DEBUG:runner:FAILED (errors=1)
DEBUG:runner:Exit Code: 1
DEBUG:bundletester.utils:Updating JUJU_ENV: "local" -> ""
ubuntu@python-django-trusty-development:/development$

on factor that may confound things

I'm calling bundletester within a container since bundletester wants a different copy of bzr than I have on my system and I need to keep my system the same. I've configured...

[Sheila Miguez (codersquid)]

On Sun, Mar 8, 2015 at 3:27 PM, Sheila Miguez <email address hidden>
wrote:

> $ bundletester -l DEBUG -o result --testdir tests

btw, I got trunk running in an amazon environment and ended up allowing
bundletester to screw up my bzr install (which I wish wouldn't happen).

bundletester -e amazon -l DEBUG -v

now trunk is failing for a better reason, which is that yaml is missing.
that's easy to figure out. I wish bundletester would run something to check
for and/or install the required dependencies.

--
<email address hidden>

[Cory Johns (johnsca)]

Sheila,

I'm sorry to hear that bundletester caused an issue with your bzr. Apparently, this is an issue that I was not aware of, and is due to bundletester being tagged to a specific, newer version of the bzr library due to a compatibility issue. I apparently have been using the newer version of bzr for a while without realizing it.

One option for working around the bzr library issue is to install bundletester inside a virtualenv:

  sudo apt-get install virtualenvwrapper
  mkvirtualenv bundletester
  pip install bundletester

This should isolate the bzr library version from the one you use on your system. Then, before running bundletester, you can use "workon bundletester" to activate the virtualenv, then run bundletester, then, to get back to your normal system env, "deactivate".

We're also working on a Docker container solution that will hopefully allow you to very easily run tests in the exact same environment that the CI will use, but unfortunately it's not quite ready for use yet.

[Marco Ceppi (marcoceppi)]

Hi Sheila, sorry again that bundletester messed up your environment. While you're working on this merge I'm going to move this to Work In Progress, when ready for another review move this to "Needs Review"

Unmerged revisions

35. By Sheila Miguez on 2015-01-07

pip install now uses the pip_extra_args settings from config.
with that in mind, --upgrade, --use-mirrors (which is deprecated), -b, and --src were all removed
Since --upgrade was set to True by default in hooks.py, I set the config default to '--upgrade'.

=== modified file 'config.yaml'
--- config.yaml	2014-11-19 17:44:03 +0000
+++ config.yaml	2015-01-07 20:21:49 +0000
@@ -93,7 +93,7 @@
           Leave the variable to an empty string if you don't want the feature.
     pip_extra_args:
         type: string
-        default: ""
+        default: "--upgrade"
         description: |
           Extra arguments passed to pip.
     requirements_apt_files:

=== modified file 'hooks/hooks.py'
--- hooks/hooks.py	2014-10-09 19:55:08 +0000
+++ hooks/hooks.py	2015-01-07 20:21:49 +0000
@@ -67,13 +67,12 @@
 # ------------------------------------------------------------------------------
 # pip_install( package ):  Installs a python package
 # ------------------------------------------------------------------------------
-def pip_install(packages=None, upgrade=False):
-    # Build in /tmp or Juju's internal git will be confused
-    cmd_line = ['pip', 'install', '-b', '/tmp/', '--src', '/usr/src/']
+def pip_install(packages=None):
+    cmd_line = ['pip', 'install']
+    if pip_extra_args:
+        cmd_line.extend(pip_extra_args.split())
     if packages is None:
         return(False)
-    if upgrade:
-        cmd_line.append('--upgrade')
     if not isinstance(packages, list):
         packages = [packages]
 
@@ -83,23 +82,20 @@
             cmd_line.append('-e')
         cmd_line.append(package)
 
-    cmd_line.append('--use-mirrors')
     return(subprocess.call(cmd_line))
 
 
 # ------------------------------------------------------------------------------
 # pip_install_req( path ):  Installs a requirements file
 # ------------------------------------------------------------------------------
-def pip_install_req(path_or_url=None, upgrade=False):
-    # Build in /tmp or Juju's internal git will be confused
-    cmd_line = ['pip', 'install', '-b', '/tmp/', '--src', '/usr/src/']
+def pip_install_req(path_or_url=None):
+    cmd_line = ['pip', 'install']
+    if pip_extra_args:
+        cmd_line.extend(pip_extra_args.split())
     if path_or_url is None:
         return(False)
-    if upgrade:
-        cmd_line.append('--upgrade')
     cmd_line.append('-r')
     cmd_line.append(path_or_url)
-    cmd_line.append('--use-mirrors')
     return(subprocess.call(cmd_line))
 
 
@@ -263,7 +259,7 @@
 
     if extra_pip_pkgs:
         for package in extra_pip_pkgs.split(','):
-            pip_install(package, upgrade=True)
+            pip_install(package)
 
     configure_and_install(django_version)
 
@@ -380,11 +376,11 @@
 
     if extra_pip_pkgs:
         for package in extra_pip_pkgs.split(','):
-            pip_install(package, upgrade=True)
+            pip_install(package)
 
     if requirements_pip_files:
         for req_file in requirements_pip_files.split(','):
-            pip_install_req(os.path.join(working_dir, req_file), upgrade=True)
+            pip_install_req(os.path.join(working_dir, req_file))
 
     run('chown -R %s:%s %s' % (wsgi_user, wsgi_group, vcs_clone_dir))
 
@@ -734,6 +730,7 @@
 extra_deb_pkgs = config_data['additional_distro_packages']
 extra_pip_pkgs = config_data['additional_pip_packages']
 requirements_pip_files = config_data['requirements_pip_files']
+pip_extra_args = config_data['pip_extra_args']
 wsgi_user = config_data['wsgi_user']
 wsgi_group = config_data['wsgi_group']
 install_root = config_data['install_root']