Skip to content
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.

Sign up for GitHub

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

Bump psycopg2 to 2.7.4; #5698

Merged
merged 8 commits into from
Mar 29, 2018
Merged

Bump psycopg2 to 2.7.4; #5698

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
1774c77
Bump psycopg2 to 2.7.4; this fixes compatibility with the default ubu…
dannon Mar 14, 2018
3b84714
Per http://initd.org/psycopg/articles/2018/02/08/psycopg-274-released/,
dannon Mar 14, 2018
3df781a
Update psycopg2 conditional dependency check to use the new psycopg2-…
dannon Mar 14, 2018
e84dd5d
Rudimentary check for old psycopg2==2.4.3, which we clean up before i…
dannon Mar 23, 2018
30c2ea0
Make psycopg2 grep a little more robust
dannon Mar 23, 2018
d44b477
Specify format to squelch deprecation warning about columns/legacy
dannon Mar 23, 2018
8742614
Quieter grepping.
dannon Mar 28, 2018
60ca4d4
Instead of using -q, send to /dev/null, since the early pipe exit blo…
dannon Mar 28, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/galaxy/dependencies/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ def check(self, name):
except Exception:
return False

def check_psycopg2(self):
def check_psycopg2_binary(self):
return self.config["database_connection"].startswith("postgres")

def check_mysql_python(self):
Expand Down
2 changes: 1 addition & 1 deletion lib/galaxy/dependencies/conditional-requirements.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# These dependencies are only required when certain config options are set
psycopg2==2.7.3
psycopg2-binary==2.7.4
Copy link
Member

@jmchilton jmchilton Mar 15, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link
Member Author

@dannon dannon Mar 28, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

WebError==0.10.3
Pygments==2.2.0
python-openid
Expand Down
8 changes: 7 additions & 1 deletion scripts/common_startup.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

grep -q

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, good idea.

Copy link
Member

@nsoranzo nsoranzo Mar 28, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Member Author

@dannon dannon Mar 28, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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)

Copy link
Member Author

@dannon dannon Mar 28, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pypa/pip#4170 is the upstream bug, if you're interested.

echo "An older version of psycopg2 (non-binary, version 2.7.3) has been detected. Galaxy now uses psycopg2-binary, which will be installed after removing psycopg2."
pip uninstall -y psycopg2==2.7.3
fi
echo "$GALAXY_CONDITIONAL_DEPENDENCIES" | pip install -r /dev/stdin --index-url "${GALAXY_WHEELS_INDEX_URL}" --extra-index-url "${PYPI_INDEX_URL}"
fi
fi

# Check client build state.
Expand Down