aio-libs / aiopg

aiopg is a library for accessing a PostgreSQL database from the asyncio
http://aiopg.readthedocs.io
BSD 2-Clause "Simplified" License
1.39k stars 157 forks source link

psycopg2-binary dependency #631

Open natewlew opened 4 years ago

natewlew commented 4 years ago

I was just wondering if psycopg2-binary is the correct dependency. Their documentation recommends that it should not be used in production: http://initd.org/psycopg/docs/install.html#binary-install-from-pypi

vir-mir commented 4 years ago

as I understand it, if we were writing an extension for psycopg this would be true. however, we repeat its parts, we do not expand the functionality, do not overlap the logic... psycopg-binary is fine in this case for task.

natewlew commented 4 years ago

I must have miss-interpreted it. Here is a direct quote:

Note The psycopg2-binary package is meant for beginners to start playing with Python and PostgreSQL without the need to meet the build requirements. If you are the maintainer of a publish package depending on psycopg2 you shouldn’t use ‘psycopg2-binary’ as a module dependency. For production use you are advised to use the source distribution.

tritium21 commented 4 years ago

I must have miss-interpreted it. Here is a direct quote:

You did not. It is against the advice of the psycopg2 developer to depend on the -binary package.

bluefish6 commented 4 years ago

+1 for replacing psycopg2-binary with psycopg2 requirement.

Helpful links:

Just to highlight the main issue with psycopg2-binary, please let me quote the author:

The resulting [psycopg2-binary] package largely works, but it has a problem: among libpq dependencies there is libssl, which is included in the wheel package. But the system libssl version may also be imported for other reason by the Python process (e.g. using 'urllib' to open an https resource). When this happens, in concurrent situations, the process may segfault.

andrewsw commented 3 years ago

Any progress on this issue? It's becoming a problem for us as aiopg versions below 1.0.0 spam our consoles with hundreds of deprecation warnings, while upgrading brings in this clearly incorrect dependency.

natewlew commented 3 years ago

We moved to asyncpg.

njsmith commented 3 years ago

Note that the only actual problem with psycopg2-binary is that psycopg2 has some Very Wrong code in it to explicitly disable thread-safety in openssl. This doesn't cause a problem if you're building from source, because both psycopg2 and the stdlib end up using the same shared copy of openssl, and the stdlib fixes up the thread-safety thing. If you're using the pre-built psycopg2 wheel, though, it includes its own private copy of openssl, and no-one fixes the thread safety after psycopg2 messes it up.

However! The whole mess with openssl thread-safety only applies to openssl 1.0.2 and earlier. In modern versions of openssl, openssl is always thread-safe, and the APIs that psycopg2 is misusing are no-ops. And openssl 1.0.2 was EOL a year ago. All supported versions of openssl are fine. (Also, I'm not sure aiopg even uses threads anyway?)

So these days, there's literally no problem at all with the psycopg2-binary wheels, except that the psycopg2 maintainers insist on giving them a different name from the main release, which creates this situation where projects like aiopg are trapped between two bad options – if aiopg depends on psycopg2-binary, it breaks alpine, but if it depends on psycopg2, then it makes the experience substantially worse for everyone on windows/macos/every other linux distro.

So you might want to pressure psycopg2 to fix this.

nicktimko commented 3 years ago

Options:

  1. aiopg depends on psycopg2-binary, it breaks alpine
  2. it depends on psycopg2, then it makes the experience substantially worse for everyone [else]

How about:

  1. don't declare it as a dependency at all in aiopg (or just using extras, e.g. pip install aiopg[psycopg2], pip install aiopg[psycopg2-binary])
n1ngu commented 2 years ago

[...] if aiopg depends on psycopg2-binary, it breaks alpine, but if it depends on psycopg2, then it makes the experience substantially worse for everyone on windows/macos/every other linux distro.

So you might want to pressure psycopg2 to fix this.

Big news, pscyopg2 moved on to psycopg3 and it should be a drop-in replacement. Then, I guess aiopg could require the lonely psycopg package for the interface and the pure-python fallback and point the users to responsibly add either psycopg[c] or psycopg[binary] depending on what is more convenient for them linking to https://www.psycopg.org/psycopg3/docs/basic/install.html . Then AFAIU psycopg3 will opportunistically choose the available implementation at import time.

This is similar to what @nicktimko suggested but surfacing the fact that psycopg3 would be used under the hood to the user, instead of exposing and documenting the same extras dependencies in aiopg.