setup.py: use wheel by default, latest psycopg2 #1059

zadacka · 2019-05-27T10:34:48Z

Motivation:

use the The psycopg2 wheel (pre-compiled binary) for convenience where possible.
allow use of psycopg2.8 (and greater!) now that it is available.

The change effectively reverts the setup.py changes in dd579cd and b95ac55, although that the 'detailed instructions' section introduced in dd579cd becomes relevant. In fact the option to install with --no-binary is only meaningful when the default requirement is the psycopg2-binary.

I think that this is a reasonable proposal, but would like to know if there were any other reasons behind b95ac55 that would prevent using psycopg2 2.8 being usable.

Description

Checklist

I've added this contribution to the changelog.rst.
I've added my name to the AUTHORS file (or it's already there).

I installed pre-commit hooks (pip install pre-commit && pre-commit install), and ran black on my code.

Motivation: * use the The psycopg2 wheel (pre-compiled binary) for convenience where possible. * allow use of psycopg2.8 (and greater!) now that it is available. The change effectively reverts the setup.py changes in dd579cd and b95ac55, although that the 'detailed instructions' section introduced in dd579cd becomes relevant. In fact the option to install with `--no-binary` is only meaningful when the default requirement is the psycopg2-binary. I think that this is a reasonable proposal, but would like to know if there were any other reasons behind b95ac55 that would prevent using psycopg2 2.8 being usable.

codecov-io · 2019-05-27T10:38:29Z

Codecov Report

Merging #1059 into master will decrease coverage by 0.12%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1059      +/-   ##
==========================================
- Coverage   84.84%   84.72%   -0.13%     
==========================================
  Files          21       21              
  Lines        2494     2494              
==========================================
- Hits         2116     2113       -3     
- Misses        378      381       +3

Impacted Files	Coverage Δ
pgcli/completion_refresher.py	`86.66% <0%> (-4%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8cb7009...e77b7d5. Read the comment docs.

zadacka · 2019-05-27T13:45:47Z

Having looked a bit further around the conversation surrounding this:
#849
#974
#844

... this PR still seems like a reasonable change (and it would close #1036).

The (recent) release of psycopg2 2.8 should get rid of the warning users have previously objected to. Depending on the wheel should be best for most users. Those who specifically need to build from source can still do so using the instructions specified in the 'detailed instructions'.

'course I may still be missing something - please let me know :)

koobs · 2019-05-28T08:40:42Z

From the psycopg docs [1]:

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.

The install_requires for this dependency should remain psycopg2, not psychopg2-binary

[1] http://initd.org/psycopg/docs/install.html#binary-install-from-pypi

pgcli depends on sqlparse<0.3.0,>=0.2.2, and sqlparse was updated to 0.3.0 in ports r495481. pgcli depends on psycopg2>=2.7.4,<2.8, and psycopg2 was updated to 2.8.2 in ports r502646. These broke run time, with the following error(s) raise DistributionNotFound(req, requirers) pkg_resources.DistributionNotFound: The 'sqlparse<0.3.0,>=0.2.2' distribution was not found and is required by pgcli raise DistributionNotFound(req, requirers) pkg_resources.DistributionNotFound: The 'psycopg2>=2.7.4,<2.8' distribution was not found and is required by pgcli Upstream upgraded sqlparse support to include 0.3.0 in PR #1052 [1], this change backports that PR accordingly. Upstream has an open PR to unpin psycopg2 [2][3] but it needs to be updated to use psycopg2 (not psycopg2-binary) in install_requires. This change patches out the psycopg2 max version. The test results shows no test failure delta with 2.8.2 over 2.7.7 (the version before ports r502646). While I'm here: - Declare pinned (maximum) RUN_DEPENDS versions according to setup.py to pick these issues up earlier during QA (max versions not being satisfied will error out during dependency builds/verification pre-commit [1] dbcli/pgcli#1052 [2] dbcli/pgcli#1059 [3] dbcli/pgcli#1036 PR: 238182 Reported by: Marcin Cieślak <saper saper info> Approved by: portmgr (blanket: run time fix, just fix it) git-svn-id: svn+ssh://svn.freebsd.org/ports/head@502867 35697150-7ecd-e111-bb59-0022644237b5

pgcli depends on sqlparse<0.3.0,>=0.2.2, and sqlparse was updated to 0.3.0 in ports r495481. pgcli depends on psycopg2>=2.7.4,<2.8, and psycopg2 was updated to 2.8.2 in ports r502646. These broke run time, with the following error(s) raise DistributionNotFound(req, requirers) pkg_resources.DistributionNotFound: The 'sqlparse<0.3.0,>=0.2.2' distribution was not found and is required by pgcli raise DistributionNotFound(req, requirers) pkg_resources.DistributionNotFound: The 'psycopg2>=2.7.4,<2.8' distribution was not found and is required by pgcli Upstream upgraded sqlparse support to include 0.3.0 in PR #1052 [1], this change backports that PR accordingly. Upstream has an open PR to unpin psycopg2 [2][3] but it needs to be updated to use psycopg2 (not psycopg2-binary) in install_requires. This change patches out the psycopg2 max version. The test results shows no test failure delta with 2.8.2 over 2.7.7 (the version before ports r502646). While I'm here: - Declare pinned (maximum) RUN_DEPENDS versions according to setup.py to pick these issues up earlier during QA (max versions not being satisfied will error out during dependency builds/verification pre-commit [1] dbcli/pgcli#1052 [2] dbcli/pgcli#1059 [3] dbcli/pgcli#1036 PR: 238182 Reported by: Marcin Cieślak <saper saper info> Approved by: portmgr (blanket: run time fix, just fix it)

pgcli depends on sqlparse<0.3.0,>=0.2.2, and sqlparse was updated to 0.3.0 in ports r495481. pgcli depends on psycopg2>=2.7.4,<2.8, and psycopg2 was updated to 2.8.2 in ports r502646. These broke run time, with the following error(s) raise DistributionNotFound(req, requirers) pkg_resources.DistributionNotFound: The 'sqlparse<0.3.0,>=0.2.2' distribution was not found and is required by pgcli raise DistributionNotFound(req, requirers) pkg_resources.DistributionNotFound: The 'psycopg2>=2.7.4,<2.8' distribution was not found and is required by pgcli Upstream upgraded sqlparse support to include 0.3.0 in PR #1052 [1], this change backports that PR accordingly. Upstream has an open PR to unpin psycopg2 [2][3] but it needs to be updated to use psycopg2 (not psycopg2-binary) in install_requires. This change patches out the psycopg2 max version. The test results shows no test failure delta with 2.8.2 over 2.7.7 (the version before ports r502646). While I'm here: - Declare pinned (maximum) RUN_DEPENDS versions according to setup.py to pick these issues up earlier during QA (max versions not being satisfied will error out during dependency builds/verification pre-commit [1] dbcli/pgcli#1052 [2] dbcli/pgcli#1059 [3] dbcli/pgcli#1036 PR: 238182 Reported by: Marcin Cieślak <saper saper info> Approved by: portmgr (blanket: run time fix, just fix it) git-svn-id: svn+ssh://svn.freebsd.org/ports/head@502867 35697150-7ecd-e111-bb59-0022644237b5

@koobs

As @koobs pointed out, the psycopg docs state that projects should depend on the source rather than the binary.

The decision to depend upon psycopg2 rather than the psycopg2-binary makes the (deleted) instructions unnecessary.

zadacka · 2019-05-28T13:55:35Z

@koobs - good point

The project seemed split on which to use, with the README making it seem like the binary was the dependency, but then the actual dependency list using the source. And historically it looks like both had been used...

I'm definitely happy to abide by the official docs and change the dependency to use the source.

Given the change to depend on source, and with the README cleanup, does this now look better to you?

koobs · 2019-05-28T14:09:31Z

@zadacka Thanks for the quick turnaround!

The real distinction is between 'user/application deployments' and 'packaging' (the same distinction as *_requires vs requirements files). [1][2] Unfortunately the distinction has been lost among the noise of cries of "terrible packaging ecosystem" over the years, made worse by everyone doing everything their own unique new way :)

But yes, I saw no test failure delta just unpinning psycopg2 in install_requires, so it should be good to go

From my FreeBSD port commit log message:

Upstream has an open PR to unpin psycopg2 [2][3] but it needs to be updated
to use psycopg2 (not psycopg2-binary) in install_requires.

This change patches out the psycopg2 max version. The test results shows
no test failure delta with 2.8.2 over 2.7.7 (the version before ports
r502646).

[1] https://packaging.python.org/discussions/install-requires-vs-requirements/
[2] https://caremad.io/posts/2013/07/setup-vs-requirement/

zadacka · 2019-05-28T14:41:50Z

@koobs Thanks! And thanks also for the links to reading notes.

Some context: I previously had some problems (now solved) when trying to install psycopg2 from source; the pip error message contained the lines:

    If you prefer to avoid building psycopg2 from source, please install the PyPI
    'psycopg2-binary' package instead.

I initially read this as 'either option is fine' which seemed to contradict the psycopg docs, but it all makes more sense to me now in the context of install_requires / requirements.

(Side note: my problem above was pg_config not being in the install user's path, fixing this allowed the installation to complete successfully and pgcli + source-built psycopg2 2.8 seems to work great so far.)

koobs · 2019-05-28T15:01:01Z

@zadacka You're welcome, and yeh, many C/extension based packages have the same issue as they overlap into system/thirdparty library land, and all the lovely challenges that come along with it like a multitude of include/library paths and supporting different compilers/toolchains. But, this is also the beauty of upstreams and downstreams working together, everyone gets to learn about the grass on the other side.

We're all consumers (downstreams) after all, as your experience too shows :)

telmotrooper · 2019-05-30T00:14:44Z

I was gonna add a merge request changing psycopg2 >= 2.7.4,<2.8 to psycopg2 >= 2.7.4,<=2.8.2, just so pgcli will work on Arch Linux without any user-made changes, but it seems you guys have it covered.

If anyone is reading this while the issue hasn't been fixed, just edit that one line in /lib/python3.7/site-packages/pgcli-2.1.0-py3.7.egg-info/requires.txt and it should start working again.

telmotrooper · 2019-05-30T00:17:27Z

Actually reading through all your conversation it seems you haven't decided on merging this. Guess I'll make the merge request with my workaround then.

Edit: here it is (#1060).

j-bennet · 2019-05-30T04:01:09Z

After digging through psycopg2 issues, it looks to me like the original segfault problem that caused the split of psycopg2 and psycopg2-binary was fixed:

psycopg/psycopg2#543

However, psycopg2-binary is still not recommended as the default choice:

psycopg/psycopg2#921 (comment)

The following PR bumps psycopg2 to 2.8.2:

#1060

I think we can merge that PR first, since jury is still out on "can wheels be default again" question.

The README update is good. The --no-binary instructions will not work with psycopg2>=2.8 anymore:

http://initd.org/psycopg/docs/install.html#change-in-binary-packages-between-psycopg-2-7-and-2-8

zadacka · 2019-05-30T07:21:23Z

~~Closing this out... this PR does two things:~~
* set the setup.py dependency "psycopg2 >= 2.7.4", (deciding against using the wheel as discussed above) - exactly the same as @telmotrooper and you settled on in #1060, so will now result in no change to trunk!
* update the README, which I think that we're agreed is sensible following the above.

~~@j-bennet would you like to merge this in order to get the README update? Or would you prefer that I submit a separate PR with just the README change?~~

For clarity, I've submitted the README changes as a separate PR: #1061

This should make it clear that this pull request (with 'use wheel by default' in the title) is NOT getting merged. It'll also keep the git history a little cleaner.

j-bennet · 2019-05-30T14:36:16Z

Yikes! Sorry, I got tricked by the PR's title, and missed the fact that you don't actually switch to psycopg2-binary. My mistake.

I'm going to review #1061.

pgcli depends on sqlparse<0.3.0,>=0.2.2, and sqlparse was updated to 0.3.0 in ports r495481. pgcli depends on psycopg2>=2.7.4,<2.8, and psycopg2 was updated to 2.8.2 in ports r502646. These broke run time, with the following error(s) raise DistributionNotFound(req, requirers) pkg_resources.DistributionNotFound: The 'sqlparse<0.3.0,>=0.2.2' distribution was not found and is required by pgcli raise DistributionNotFound(req, requirers) pkg_resources.DistributionNotFound: The 'psycopg2>=2.7.4,<2.8' distribution was not found and is required by pgcli Upstream upgraded sqlparse support to include 0.3.0 in PR #1052 [1], this change backports that PR accordingly. Upstream has an open PR to unpin psycopg2 [2][3] but it needs to be updated to use psycopg2 (not psycopg2-binary) in install_requires. This change patches out the psycopg2 max version. The test results shows no test failure delta with 2.8.2 over 2.7.7 (the version before ports r502646). While I'm here: - Declare pinned (maximum) RUN_DEPENDS versions according to setup.py to pick these issues up earlier during QA (max versions not being satisfied will error out during dependency builds/verification pre-commit [1] dbcli/pgcli#1052 [2] dbcli/pgcli#1059 [3] dbcli/pgcli#1036 PR: 238182 Reported by: Marcin Cieślak <saper saper info> Approved by: portmgr (blanket: run time fix, just fix it)

zadacka added 2 commits May 28, 2019 14:47

setup.py: revert dependency psycopg2 (no-binary)

ff3686f

As @koobs pointed out, the psycopg docs state that projects should depend on the source rather than the binary.

README.rst: remove redundant wheel instructions

e77b7d5

The decision to depend upon psycopg2 rather than the psycopg2-binary makes the (deleted) instructions unnecessary.

j-bennet mentioned this pull request May 30, 2019

Allow psycopg2 up to 2.8.2 #1060

Merged

3 tasks

zadacka closed this May 30, 2019

zadacka deleted the patch-1 branch May 30, 2019 09:00

j-bennet mentioned this pull request May 30, 2019

README.rst: tidy up redundant instructions #1061

Merged

3 tasks

j-bennet mentioned this pull request Aug 2, 2019

move to psycopg2-binary? #1080

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

setup.py: use wheel by default, latest psycopg2 #1059

setup.py: use wheel by default, latest psycopg2 #1059

zadacka commented May 27, 2019

codecov-io commented May 27, 2019 •

edited

zadacka commented May 27, 2019 •

edited

koobs commented May 28, 2019

zadacka commented May 28, 2019

koobs commented May 28, 2019

zadacka commented May 28, 2019

koobs commented May 28, 2019 •

edited

telmotrooper commented May 30, 2019

telmotrooper commented May 30, 2019 •

edited

j-bennet commented May 30, 2019

zadacka commented May 30, 2019 •

edited

j-bennet commented May 30, 2019

setup.py: use wheel by default, latest psycopg2 #1059

setup.py: use wheel by default, latest psycopg2 #1059

Conversation

zadacka commented May 27, 2019

Description

Checklist

codecov-io commented May 27, 2019 • edited

Codecov Report

zadacka commented May 27, 2019 • edited

koobs commented May 28, 2019

zadacka commented May 28, 2019

koobs commented May 28, 2019

zadacka commented May 28, 2019

koobs commented May 28, 2019 • edited

telmotrooper commented May 30, 2019

telmotrooper commented May 30, 2019 • edited

j-bennet commented May 30, 2019

zadacka commented May 30, 2019 • edited

j-bennet commented May 30, 2019

codecov-io commented May 27, 2019 •

edited

zadacka commented May 27, 2019 •

edited

koobs commented May 28, 2019 •

edited

telmotrooper commented May 30, 2019 •

edited

zadacka commented May 30, 2019 •

edited