Bump psycopg2 to 2.7.4; #5698
Bump psycopg2 to 2.7.4; #5698
Conversation
…ntu 18.04 postgresql
|
The instructions on pypi are a bit strange, but if we can trust them we'd actually need to use |
|
@mvdbeek yeah, reading through http://initd.org/psycopg/articles/2018/02/08/psycopg-274-released/ now to try to grok this. |
swap to psycopg2-binary package for 2.7.4 (and future) wheels.
…binary package name.
| @@ -1,5 +1,5 @@ | |||
| # These dependencies are only required when certain config options are set | |||
| psycopg2==2.7.3 | |||
| psycopg2-binary==2.7.4 | |||
There was a problem hiding this comment.
If we change the name of this - it won't upgrade (edit: uninstall might be a better word) older versions of this when upgrading Galaxy right? This could still be the right thing to do but maybe we need to think about how to handle that - should we uninstall psycopg2 in common startup?
There was a problem hiding this comment.
@jmchilton Good point, I can check what happens but I don't know how this would behave off the top of my head if you have both installed.
There was a problem hiding this comment.
To update this comment, which was subsequently discussed on IRC - the suspicion was correct. Installing psycopg2-binary does not automatically uninstall psycopg2 even though they share the same namespace in site-packages. This leaves you in a weird situation where pip thinks both are installed, but really only the most recently installed library is intact.
Uninstalling the previous lib will mangle the new one, and so taking care to manually remove psycopg2==2.4.3 and removing it prior to installing a new psycopg2-binary version, when in galaxy's .venv, is what I've implemented in subsequent commits. This seems the safest path.
This behavior should mirror what pip would do if we had a new version of psycopg2==2.4.3 listed here -- that is, uninstalling the old prior to installing new.
|
I took a stab at checking/removing old psycopg2, this should be ready to go. |
scripts/common_startup.sh
Outdated
| @@ -186,7 +186,13 @@ fi | |||
| if [ $FETCH_WHEELS -eq 1 ]; then | |||
| pip install $requirement_args --index-url "${GALAXY_WHEELS_INDEX_URL}" --extra-index-url "${PYPI_INDEX_URL}" | |||
| GALAXY_CONDITIONAL_DEPENDENCIES=$(PYTHONPATH=lib python -c "from __future__ import print_function; import galaxy.dependencies; print('\n'.join(galaxy.dependencies.optional('$GALAXY_CONFIG_FILE')))") | |||
| [ -z "$GALAXY_CONDITIONAL_DEPENDENCIES" ] || echo "$GALAXY_CONDITIONAL_DEPENDENCIES" | pip install -r /dev/stdin --index-url "${GALAXY_WHEELS_INDEX_URL}" --extra-index-url "${PYPI_INDEX_URL}" | |||
| if [ -n "$GALAXY_CONDITIONAL_DEPENDENCIES" ]; then | |||
| if pip list --format=columns | grep "psycopg2[\(\ ]*2.7.3"; then | |||
There was a problem hiding this comment.
This actually somehow make pip blow out on my shell!
(.venv) $ pip list --format=columns | grep -q "psycopg2[\(\ ]*2.7.3"
Traceback (most recent call last):
File "/usr/lib/python2.7/logging/__init__.py", line 885, in emit
self.flush()
File "/usr/lib/python2.7/logging/__init__.py", line 845, in flush
self.stream.flush()
IOError: [Errno 32] Broken pipe
Logged from file list.py, line 272
...
(edit: only with the -q)
There was a problem hiding this comment.
@nsoranzo Very strange... Can you 'pip list' at all, manually from the .venv?
Edit: Or are you saying that the addition of -q is causing the blowup for you? (yes, it was the -q)
There was a problem hiding this comment.
I see the same thing here. https://unix.stackexchange.com/questions/305547/broken-pipe-when-grepping-output-but-only-with-i-flag seems to explain it.
I can either leave out -q, or do something like:
grep "psycopg2..." >/dev/null
There was a problem hiding this comment.
@dannon See edited message above, adding the -q causes the error messages. Same behaviour with e.g. pip show psycopg2 | grep -q '^Version: 2.7.3$' . Adding the redirect to /dev/null works fine for me, go for it!
There was a problem hiding this comment.
pypa/pip#4170 is the upstream bug, if you're interested.
Fixes #5696
Pypi has wheels ready to go, so we can swap to tracking those instead of building our own.
@natefoo Just adding them to cargo-port and leaving starforge as-is should be all I need to do after this, right?