1074861 - installing a pypi-hosted requirement on top of a tarball-hosted requirement creates a frankenstein

Status: RESOLVED FIXED

(support.mozilla.org :: Code Quality, task, P1)

Description

[Will Kahn-Greene [:willkg] ET needinfo? me]

In the requirements file, elasticutils is installed from a github-hosted tarball:

# elasticutils: tags/v0.9.1^0
# sha256: OTZ4mjjiLzXDRK7mbZ5hzOSR7v62NYpUYS_FYNKBoPo
https://github.com/mozilla/elasticutils/archive641ccade9cfba1415809077fad674b41b436d174.tar.gz#egg=elasticutils

If I change that to this:

# sha256: E_91_bau0KJSZ6_JIBTR8b2alaakyi1OXquychsnX4Y
elasticutils==0.9.1

and then do a:

./scripts/peep.py install -r requirements/py26.txt

which is what we do in a deploy, then what we end up with is this:

~/.virtualenvs/kitsune/lib/python2.6/site-packages> ls -l elasticutils*
elasticutils:
total 192
drwxrwxr-x. 3 willkg willkg  4096 Sep 30 09:52 contrib
-rw-rw-r--. 1 willkg willkg  4499 Sep 30 10:04 estestcase.py
-rw-rw-r--. 1 willkg willkg  5840 Sep 30 10:04 estestcase.pyc
-rw-rw-r--. 1 willkg willkg 71341 Sep 30 10:04 __init__.py
-rw-rw-r--. 1 willkg willkg 72199 Sep 30 10:04 __init__.pyc
-rw-rw-r--. 1 willkg willkg   979 Sep 30 10:04 monkeypatch.py
-rw-rw-r--. 1 willkg willkg  1500 Sep 30 10:04 monkeypatch.pyc
drwxrwxr-x. 2 willkg willkg  4096 Sep 30 09:52 tests
-rw-rw-r--. 1 willkg willkg  1968 Sep 30 10:04 utils.py
-rw-rw-r--. 1 willkg willkg  2172 Sep 30 10:04 utils.pyc
-rw-rw-r--. 1 willkg willkg   177 Sep 30 10:04 _version.py
-rw-rw-r--. 1 willkg willkg   207 Sep 30 10:04 _version.pyc

elasticutils-0.10.1.dist-info:
total 24
-rw-rw-r--. 1 willkg willkg  996 Sep 30 10:04 DESCRIPTION.rst
-rw-rw-r--. 1 willkg willkg 2004 Sep 30 10:04 METADATA
-rw-rw-r--. 1 willkg willkg 1089 Sep 30 10:04 metadata.json
-rw-rw-r--. 1 willkg willkg 3865 Sep 30 10:04 RECORD
-rw-rw-r--. 1 willkg willkg   13 Sep 30 10:04 top_level.txt
-rw-rw-r--. 1 willkg willkg   92 Sep 30 10:04 WHEEL

elasticutils-0.9.1-py2.6.egg-info:
total 24
-rw-rw-r--. 1 willkg willkg    1 Sep 30 09:52 dependency_links.txt
-rw-rw-r--. 1 willkg willkg 2002 Sep 30 09:52 installed-files.txt
-rw-rw-r--. 1 willkg willkg 2112 Sep 30 09:52 PKG-INFO
-rw-rw-r--. 1 willkg willkg   29 Sep 30 09:52 requires.txt
-rw-rw-r--. 1 willkg willkg 1796 Sep 30 09:52 SOURCES.txt
-rw-rw-r--. 1 willkg willkg   13 Sep 30 09:52 top_level.txt

The packaging files for the old version are still there and point to elasticutils. It looks like peep/pip installs the new version *on top of* the old version without first deleting files.

Over time as we update requirements, this will result in virtualenvs on the production servers that are "frankensteins" with leftover files and .pyc files from previous versions.

Comment 1

[Will Kahn-Greene [:willkg] ET needinfo? me]

I redid this test using just stuff hosted from pypi:

   # remove all the elasticutils stuff from the virtualenv
   $ pip install elasticutils==0.9.1
   $ pip install elasticutils==0.10.1

Then I check the virtualenv and there's no evidence of 0.9.1 floating around--everything is 0.10.1.

Thus, I'm pretty sure this is just a problem that occurs with github-hosted tarballs.

Comment 2

[Will Kahn-Greene [:willkg] ET needinfo? me]

I miffed the STR in the description. STR (with more details) are these:

1. checkout master
2. create a fresh virtual env
3. do ./scripts/peep.py install -r requirements/py26.txt

   This installs elasticutils 0.9.1 from a github tarball.

4. then change the elasticutils section in requirements/default.txt to:

   # sha256: CnOPMnGcdnU38yYRjBBcIZ5nfna3hUfthHxqqvvKDGU
   elasticutils==0.10.1

5. do ./scripts/peep.py install -r requirements/py26.txt

That creates the directory listings seen in the description.

Comment 3

[Michael Cooper [:mythmon]]

I did some research. I learned that pip does not exhibit this behavior, so it is a peep bug, not pip. This doesn't help us in the short term, but I filed an issue against peep [0] for this.

[0]: https://github.com/erikrose/peep/issues/50

Comment 4

[Will Kahn-Greene [:willkg] ET needinfo? me]

This covers my poking around based on Mike's awesome sleuthing:

https://github.com/erikrose/peep/issues/50#issuecomment-57472629

tl;dr is that we add a --no-use-wheel argument to our peep command. That makes the problem go away. Alternatively, since we've got our own version of peep, we can just codify it in peep. That's probably a better solution since it means we don't have to remember to add that argument every time we run peep.

I think I'm going to do that with fjord.

Mike: Thoughts? Do you think it'd be ok to just add --no-use-wheel to the arguments peep always passes pip?

Comment 5

[Michael Cooper [:mythmon]]

I like the idea of adding --no-use-wheel to our deployment script for now, until we get on (and there exists) a fixed version of peep. I'm not a fan of making our copy of peep diverge from the peep on pypi, but it seems like we might have a fix in Peep soon, so maybe that won't matter.

To that end, I filed a PR: https://github.com/mozilla/kitsune/pull/2146

Comment 6

[Will Kahn-Greene [:willkg] ET needinfo? me]

If it helps, I did this in fjord already:

https://github.com/mozilla/fjord/commit/c726b06780cf5b5d9f86b267f1526c0fd57ee6df

I'll look at your PR today.

Comment 7

[Michael Cooper [:mythmon]]

Landed in: https://github.com/mozilla/kitsune/commit/a1b32036209781c555285675e4d53e76408400e3

Leaving this open until we deploy on Monday.

Comment 8

[Ricky Rosario [:rrosario, :r1cky]]

assigning and stuff

Comment 9

[Ricky Rosario [:rrosario, :r1cky]]

deployed to prod

Comment 10

[Will Kahn-Greene [:willkg] ET needinfo? me]

The changes in this bug got reverted because the version of pip on the admin node is 1.3.something. Because it's 1.3.something, it doesn't do wheels at all, so the theory is that we don't have this problem in our server environments.

Further, I was working on a patch to peep, but then we collectively decided we don't want to make the change there. So that's not going to happen anymore.

Thus, we still have this problem in dev environments, so I'm going to reopen this.

Possible solutions:

1. add --no-use-wheel to the documentation and send an email making sure everyone knows about it (we should be sending emails whenever we change anything regarding developer workflow)

2. modify peep locally to detect the pip version and apply --no-use-wheel when appropriate (>= 1.5.0 and < 1.6)

3. some third alternative I haven't thought of

Comment 11

[Michael Cooper [:mythmon]]

I added some docs about using --no-use-wheel when using peep, and send an email outlining the situation to sumo-eng@ and sumo-dev@.

Comment 12

[Ed Morley [:emorley]]

It seems like this is now fixed in peep (or more peep adds the workaround itself if you are using an affected version of pip):
https://github.com/willkg/peep/commit/e45893959d3e2009fb2f29a68a1cf6bb113ea1b5

As such the docs can probably be updated (re comment 11) and the "--no-use-wheel" removed from here:
https://github.com/mozilla/kitsune/blob/23f5fb39b5e35dea0a54057d7083716c4ad3a82f/scripts/update/deploy.py#L123

(Stumbled across this since Treeherder is about to start using peep so was being nosy as to what other people's Chief deploy scripts did)

Comment 13

[Will Kahn-Greene [:willkg] ET needinfo? me]

Ed: That PR was closed out and never merged because the various versions of pip have different flags and defaults in regards to wheels. Plus some of them don't work with wheels at all. It was just too difficult to make it work right for most/all cases.

For Fjord, I wrote a shell script which calls peep.py:

https://github.com/mozilla/fjord/blob/master/peep.sh

That checks the version of pip and adds the appropriate flags. I've only tested it with the versions of pip I'm using plus what gets installed in the Vagrant environment.

Comment 14

[Ed Morley [:emorley]]

(In reply to Will Kahn-Greene [:willkg] from comment #13)
> Ed: That PR was closed out and never merged 

Ah I missed that it had never been merged. Makes sense now :-)