Pip 6.0+ compatibility fixes #66

jgmize · 2014-12-29T16:57:25Z

Use stdlib logger in newer versions of pip
See pypa/pip#2008 for details

Use stdlib logger in newer versions of pip See pypa/pip#2008 for details

jgmize · 2014-12-29T16:57:59Z

Fixes #65

willkg · 2014-12-29T20:21:47Z

Seeing a lot of these failures in the tests:

Traceback (most recent call last):

File "/home/travis/build/erikrose/peep/peep.py", line 807, in <module>

exit(main())

File "/home/travis/build/erikrose/peep/peep.py", line 798, in main

return commands[argv[1]](argv[2:])

File "/home/travis/build/erikrose/peep/peep.py", line 758, in peep_install

for path in req_paths))

File "/home/travis/build/erikrose/peep/peep.py", line 758, in <genexpr>

for path in req_paths))

File "/home/travis/build/erikrose/peep/peep.py", line 734, in downloaded_reqs_from_path

parse_requirements(path, options=EmptyOptions())]

File "/home/travis/build/erikrose/peep/peep.py", line 733, in <listcomp>

return [DownloadedReq(req, argv) for req in

File "/home/travis/virtualenv/python3.4.2/lib/python3.4/site-packages/pip/req/req_file.py", line 19, in parse_requirements

"parse_requirements() missing 1 required keyword argument: "

TypeError: parse_requirements() missing 1 required keyword argument: 'session'

@jgmize Did you see these when you ran the tests with your fix?

willkg · 2014-12-29T20:23:25Z

peep.py

+try:
+    from pip.log import logger
+except ImportError:
+    from pip import logger  # https://github.com/pypa/pip/pull/2008


Do you know offhand which version of pip this is related to? It's probably more useful to comment with the pip version than the pull request url.

@erikrose Opinions on that? ^^^

I'm guessing 6.0, but I haven't verified that yet. Happy to update once I've confirmed.

jgmize · 2014-12-30T04:58:26Z

@jgmize Did you see these when you ran the tests with your fix?

No, I saw a different error when I ran tox: tox.ConfigError: ConfigError: substitution key '26,27' not found which I assumed was a local config issue, so I just ran nosetests locally in a single py27 virtualenv. All tests passed, but that was only with py27 and pip 1.5.4. I figured it was easier to just submit the PR and let travis tell us if there were other issues, which unfortunately there are. It looks like the ImportError was merely the tip of the incompatible iceberg here :(

jgmize · 2014-12-30T05:05:50Z

I've replicated the errors locally now (still not with tox, just did a pip install --upgrade pip in my py27 venv) and I'm looking into what it will take to fix them without breaking backwards compatibility.

Pass now-required session to parse_requirements Add isolated_mode attribute to EmptyOptions class

jgmize · 2014-12-30T05:32:11Z

Looks like the other failures are due to pypa/pip#1796 which made the session kwarg a requirement, but the kwarg was there before. For now, I've added a session=PipSession() (imported from pip.download) which got me to the next error complaining about a missing isolated_mode attr on the EmptyOptions fake. I've added that, and the tests pass locally with the latest version of pip, but I'm not sure that the session kwarg on parse_requirements always existed or if we'll need some additional logic for backwards compat.

jgmize · 2014-12-30T05:36:13Z

Well, travis is happy now, but after some additional testing I found that pip.download was not available in pip 0.6.2, which appears to be the oldest supported version, so I'll add that logic now.

jgmize · 2014-12-30T06:27:21Z

I've confirmed that this all started with 6.0 (6.0.3 is latest release as of this writing) and updated and added comments to that effect in the code.

willkg · 2014-12-30T13:06:26Z

I really appreciate the work you're putting into this. I'll go through this today.

willkg · 2014-12-31T21:01:37Z

peep.py

+try:
+    from pip.log import logger
+except ImportError:
+    from pip import logger  # 6.0


I thought this was a typo, but then discovered they jumped from 1.5.6 to 6.0. Yay versioning!

willkg · 2014-12-31T21:20:59Z

I tweaked my tox.ini file (I'll submit a PR for that after this lands) and ran tox and everything is fine except Python 3.2 and 3.4 interpreter environments because I don't have 3.2 and 3.4 on my machine.

Everything here looks good to me.

@erikrose Do you want to eyeball this or is my r+ good enough?

erikrose · 2015-01-01T20:18:12Z

peep.py

+    try:
+        return [DownloadedReq(req, argv) for req in
+                parse_requirements(path, options=EmptyOptions())]
+    except TypeError:


I think it would be safer to try the import and catch the ImportError rather than catching TypeError for such a broad piece of code. We could even build a kwargs dict then, avoiding the repeated list comp. Thank you for the fix!

(I'm assuming the import succeeds if and only if session is a required arg. If not, then never mind.)

No, I believe you could import PipSession from pip.download before session became a required kwarg, I'm just not sure exactly what version of pip parse_requirements began accepting the session kwarg, so it seemed safer to use the original call that we knew worked up to pip 1.5.6 and only pass in a session if it complains with the TypeError.

erikrose · 2015-01-01T20:20:35Z

I did happen to eyeball it, but in the future, yes, your r+ is plenty good enough!

erikrose · 2015-01-01T20:22:57Z

I see we don't have Travis running the full Cartesian product of pip and Python versions tox does. We should do something about that sometime.

erikrose · 2015-01-05T03:57:30Z

In case I was unclear, merge away. Thanks for the patch, @jgmize!

willkg · 2015-01-05T14:40:05Z

issue #67 covers fixing travis to run all the tests.

issue #68 covers updating tox.ini to cover pip 6.0, 6.0.1 and 6.0.2.

I think that's everything outstanding here. Merging!

Pip 6.0+ compatibility fixes

Handle ImportError due to removal of pip.log

55da5e5

Use stdlib logger in newer versions of pip See pypa/pip#2008 for details

willkg reviewed Dec 29, 2014
View reviewed changes

Additional fixes for pip 6.0+

1c0a0a7

Pass now-required session to parse_requirements Add isolated_mode attribute to EmptyOptions class

jgmize added 2 commits December 29, 2014 23:46

Add error handling for backwards compat

1057829

Update and add comments about pip 6.0+

f92ec98

jgmize changed the title ~~Handle ImportError due to removal of pip.log~~ Pip 6.0 compatibility fixes Dec 30, 2014

jgmize changed the title ~~Pip 6.0 compatibility fixes~~ Pip 6.0+ compatibility fixes Dec 30, 2014

willkg reviewed Dec 31, 2014
View reviewed changes

erikrose reviewed Jan 1, 2015
View reviewed changes

willkg added a commit that referenced this pull request Jan 5, 2015

Merge pull request #66 from jgmize/handle-logger-import-error

6427be0

Pip 6.0+ compatibility fixes

willkg merged commit 6427be0 into erikrose:master Jan 5, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pip 6.0+ compatibility fixes #66

Pip 6.0+ compatibility fixes #66

jgmize commented Dec 29, 2014

jgmize commented Dec 29, 2014

willkg commented Dec 29, 2014

willkg Dec 29, 2014

jgmize Dec 30, 2014

jgmize commented Dec 30, 2014

jgmize commented Dec 30, 2014

jgmize commented Dec 30, 2014

jgmize commented Dec 30, 2014

jgmize commented Dec 30, 2014

willkg commented Dec 30, 2014

willkg Dec 31, 2014

willkg commented Dec 31, 2014

erikrose Jan 1, 2015

erikrose Jan 2, 2015

jgmize Jan 2, 2015

erikrose commented Jan 1, 2015

erikrose commented Jan 1, 2015

erikrose commented Jan 5, 2015

willkg commented Jan 5, 2015

Pip 6.0+ compatibility fixes #66

Pip 6.0+ compatibility fixes #66

Conversation

jgmize commented Dec 29, 2014

jgmize commented Dec 29, 2014

willkg commented Dec 29, 2014

willkg Dec 29, 2014

Choose a reason for hiding this comment

jgmize Dec 30, 2014

Choose a reason for hiding this comment

jgmize commented Dec 30, 2014

jgmize commented Dec 30, 2014

jgmize commented Dec 30, 2014

jgmize commented Dec 30, 2014

jgmize commented Dec 30, 2014

willkg commented Dec 30, 2014

willkg Dec 31, 2014

Choose a reason for hiding this comment

willkg commented Dec 31, 2014

erikrose Jan 1, 2015

Choose a reason for hiding this comment

erikrose Jan 2, 2015

Choose a reason for hiding this comment

jgmize Jan 2, 2015

Choose a reason for hiding this comment

erikrose commented Jan 1, 2015

erikrose commented Jan 1, 2015

erikrose commented Jan 5, 2015

willkg commented Jan 5, 2015