-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Updating _run_get_stderr() to not break on failed installs from pip() #35
Conversation
…dict of errored installs instead of breaking on list of install packages
After talking with Bill today, I tested the changes above with GLOB sdist-make: /Users/djshoup/Desktop/ipydeps/setup.py
py27 recreate: /Users/djshoup/Desktop/ipydeps/.tox/py27
py27 installdeps: discover
py27 inst: /Users/djshoup/Desktop/ipydeps/.tox/.tmp/package/1/ipydeps-0.8.1.zip
py27 installed: DEPRECATION: Python 2.7 will reach the end of its life on January 1st, 2020. Please upgrade your Python as Python 2.7 won't be maintained after that date. A future version of pip will drop support for Python 2.7.,asn1crypto==0.24.0,cffi==1.11.5,cryptography==2.5,discover==0.4.0,enum34==1.1.6,ipaddress==1.0.22,ipydeps==0.8.1,pycparser==2.19,pyOpenSSL==19.0.0,pypki2==0.10.3,six==1.12.0,tqdm==4.30.0
py27 run-test-pre: PYTHONHASHSEED='709577510'
py27 runtests: commands[0] | discover
........2019-01-29 20:54:33,662 - WARNING - Duplicate package name foo in dependencies JSON. Package names are case-insensitive. Overwriting!
Duplicate package name foo in dependencies JSON. Package names are case-insensitive. Overwriting!
2019-01-29 20:54:33,662 - WARNING - Duplicate package name foo in dependencies JSON. Package names are case-insensitive. Overwriting!
.......DEPRECATION: Python 2.7 will reach the end of its life on January 1st, 2020. Please upgrade your Python as Python 2.7 won't be maintained after that date. A future version of pip will drop support for Python 2.7.
....
----------------------------------------------------------------------
Ran 19 tests in 0.633s
OK
py27 runtests: commands[1] | python -m ipydeps six,geohash
DEPRECATION: Python 2.7 will reach the end of its life on January 1st, 2020. Please upgrade your Python as Python 2.7 won't be maintained after that date. A future version of pip will drop support for Python 2.7.
Packages already installed: six
DEPRECATION: Python 2.7 will reach the end of its life on January 1st, 2020. Please upgrade your Python as Python 2.7 won't be maintained after that date. A future version of pip will drop support for Python 2.7.
DEPRECATION: Python 2.7 will reach the end of its life on January 1st, 2020. Please upgrade your Python as Python 2.7 won't be maintained after that date. A future version of pip will drop support for Python 2.7.
Running pip to install geohash
DEPRECATION: Python 2.7 will reach the end of its life on January 1st, 2020. Please upgrade your Python as Python 2.7 won't be maintained after that date. A future version of pip will drop support for Python 2.7.
No new packages installed
Done
py27 runtests: commands[2] | python -m ipydeps urllib exifread
DEPRECATION: Python 2.7 will reach the end of its life on January 1st, 2020. Please upgrade your Python as Python 2.7 won't be maintained after that date. A future version of pip will drop support for Python 2.7.
urllib is part of the Python standard library and will be skipped. Remove it from the list to remove this warning.
DEPRECATION: Python 2.7 will reach the end of its life on January 1st, 2020. Please upgrade your Python as Python 2.7 won't be maintained after that date. A future version of pip will drop support for Python 2.7.
DEPRECATION: Python 2.7 will reach the end of its life on January 1st, 2020. Please upgrade your Python as Python 2.7 won't be maintained after that date. A future version of pip will drop support for Python 2.7.
Running pip to install exifread
DEPRECATION: Python 2.7 will reach the end of its life on January 1st, 2020. Please upgrade your Python as Python 2.7 won't be maintained after that date. A future version of pip will drop support for Python 2.7.
No new packages installed
Done
py35 create: /Users/djshoup/Desktop/ipydeps/.tox/py35
py35 installdeps: discover
py35 inst: /Users/djshoup/Desktop/ipydeps/.tox/.tmp/package/1/ipydeps-0.8.1.zip
py35 installed: asn1crypto==0.24.0,cffi==1.11.5,cryptography==2.5,discover==0.4.0,ipydeps==0.8.1,pycparser==2.19,pyOpenSSL==19.0.0,pypki2==0.10.3,six==1.12.0,tqdm==4.30.0
py35 run-test-pre: PYTHONHASHSEED='709577510'
py35 runtests: commands[0] | discover
........2019-01-29 20:55:15,728 - WARNING - Duplicate package name foo in dependencies JSON. Package names are case-insensitive. Overwriting!
Duplicate package name foo in dependencies JSON. Package names are case-insensitive. Overwriting!
2019-01-29 20:55:15,728 - WARNING - Duplicate package name foo in dependencies JSON. Package names are case-insensitive. Overwriting!
...........
----------------------------------------------------------------------
Ran 19 tests in 0.800s
OK
py35 runtests: commands[1] | python -m ipydeps six,geohash
Packages already installed: six
Running pip to install geohash
No new packages installed
Done
py35 runtests: commands[2] | python -m ipydeps urllib exifread
urllib is part of the Python standard library and will be skipped. Remove it from the list to remove this warning.
Running pip to install exifread
No new packages installed
Done
py37 create: /Users/djshoup/Desktop/ipydeps/.tox/py37
py37 installdeps: discover
py37 inst: /Users/djshoup/Desktop/ipydeps/.tox/.tmp/package/1/ipydeps-0.8.1.zip
py37 installed: asn1crypto==0.24.0,cffi==1.11.5,cryptography==2.5,discover==0.4.0,ipydeps==0.8.1,pycparser==2.19,pyOpenSSL==19.0.0,pypki2==0.10.3,six==1.12.0,tqdm==4.30.0
py37 run-test-pre: PYTHONHASHSEED='709577510'
py37 runtests: commands[0] | discover
........2019-01-29 20:55:47,021 - WARNING - Duplicate package name foo in dependencies JSON. Package names are case-insensitive. Overwriting!
Duplicate package name foo in dependencies JSON. Package names are case-insensitive. Overwriting!
2019-01-29 20:55:47,021 - WARNING - Duplicate package name foo in dependencies JSON. Package names are case-insensitive. Overwriting!
...........
----------------------------------------------------------------------
Ran 19 tests in 0.494s
OK
py37 runtests: commands[1] | python -m ipydeps six,geohash
Packages already installed: six
Running pip to install geohash
No new packages installed
Done
py37 runtests: commands[2] | python -m ipydeps urllib exifread
urllib is part of the Python standard library and will be skipped. Remove it from the list to remove this warning.
Running pip to install exifread
No new packages installed
Done
_________________________________________________________________________________________________ summary __________________________________________________________________________________________________
py27: commands succeeded
py35: commands succeeded
py37: commands succeeded
congratulations :) |
ipydeps/__init__.py
Outdated
elif sys.version_info.major == 2: | ||
err = e.output | ||
|
||
packages = cmd[5:] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do we know the package names start a position 5?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one I may need some help testing, since I'm not sure what adjustments to the _pip_exec
args would result in package names being somewhere else, but this is what I was looking at with some print-debugging _pip_exec
and _run_get_stderr
:
Do you guys have any suggested ways of testing other optional arguments? Or would that depend on what _config_options
are passed in?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking about this a bit more, there's already a nice list of packages in pip() on line 441. It might make more sense to move the loop and progress bar code out to that level (and wrap it in a function to keep things neater).
ipydeps/__init__.py
Outdated
return returncode, err | ||
progress_bar = tqdm.tqdm(total=len(packages), desc="Installing...") | ||
|
||
for package in packages: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My biggest concern with the iterative approach is that pip does some things behind the scenes when determining the order to install a list of packages:
https://github.com/pypa/pip/blob/master/src/pip/_internal/resolve.py#L367
called from
https://github.com/pypa/pip/blob/master/src/pip/_internal/commands/install.py#L366
Granted, we may already be undermining this with overrides from dependencies.json.
…s summary dict to not show successfully installed packages
@@ -428,19 +428,21 @@ def pip(pkg_name, verbose=False, progress=False): | |||
pip_exec = _get_pip_exec(_config_options) | |||
package_errs = {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gershwinlabs After a couple of adjustments, I like your idea of bringing the package iteration and progress bar out to pip() (much cleaner!) -- as for wrapping it up in a function for added cleanliness, do you mean lines 429-436 in this commit?
I merged your PR into the issue34 branch of ipydeps and made some more modifications to separate the original installation path from the new tqdm path. This should allow us to experiment with the iterative approach, while not breaking anything that's currently using ipydeps. All the old tests pass. Can you try it out and see if it still works as you expect when you set |
...and to return a dict of
{package : (returncode, err)}
pairs for_log_before_after
output if exception is raised.Example:
Also adding sample tqdm progress bar for showing install progress.
For issue 34: #34