#24335 closed Cleanup/optimization (fixed)
DateRange and friends require psycopg2 >= 2.5
Reported by: | Claude Paroz | Owned by: | Tim Graham |
---|---|---|---|
Component: | contrib.postgres | Version: | 1.8alpha1 |
Severity: | Normal | Keywords: | 1.8-beta |
Cc: | Marc Tamlyn, ovangle@… | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Currently, if your psycopg2 version is lower than 2.5, you cannot import any of contrib.postgres fields or forms, as ranges.py is unconditionally imported in their __init__
.
On one side, this should be documented, and on the other side we should still allow other fields/forms (array/hstore) to be imported (maybe raise a RuntimeWarning for the ranges import failure?).
Change History (14)
comment:1 by , 10 years ago
Cc: | added |
---|---|
Keywords: | 1.8-beta added |
Triage Stage: | Unreviewed → Accepted |
comment:2 by , 10 years ago
Cc: | added |
---|---|
Owner: | set to |
Status: | new → assigned |
comment:3 by , 10 years ago
comment:4 by , 10 years ago
I've submitted a patch https://github.com/django/django/pull/4131 which conditionally defines the classes if an ImportError
is raised when importing psycopg2.extras.Range.
I'm happy changing it to require psycopg2>=2.5
, but obviously since this is my first contribution a suggestion like that was not my call to make and the bug as described had been accepted, so I implemented it directly as stated. Personally, my opinion on these matters is usually "continue support for the last two major versions of a package".
comment:5 by , 10 years ago
Psycopg2 2.6 was released last week - http://initd.org/psycopg/articles/2015/02/09/psycopg-26-and-255-released/
comment:6 by , 10 years ago
I was aware of that, but I meant the two previous major versions (so, not including the current version). It's arbitrary anyway and since this is entirely new optional module, it's a little early to be littering the code with conditional imports.
>= 2.5
is OK with me.
comment:7 by , 10 years ago
I was aware of that, but I meant the two previous major versions (so, not including the current version). It's arbitrary anyway and since this is entirely new functionality in an optional module, it's a little early to be littering the code with conditional imports.
>= 2.5
is OK with me.
comment:8 by , 10 years ago
Requiring psycopg2>=2.5
would literally force many users to use a pip-installed psycopg2
instead of the system one, as many stable distributions still have an older version. But if we agree that's an acceptable requirement for a contrib package, then go for it (and document it, of course).
comment:9 by , 10 years ago
While testing this, it seems that https://github.com/django/django/commit/39d95fb6ada99c59d47fa0eae6d3128abafe2d58 dropped support for psycopg2 <= 2.4.3 globally (new_array_type was added in 2.4.3). I'm just going to add a check for the version >= 2.5 into django.db.backends.postgres_pyscopg2
and update docs/ref/databases.txt
.
edit: I posted this before reading your comment @claudep (it has been sitting unposted in my browser window for a while now). Perhaps this needs more discussion.
comment:10 by , 10 years ago
The relevant array code added in that commit is actually only needed for contrib.postgres in theory, but it might be quite complicated to conditionally include it as it also changes the way GenericIPAddressField
works, I think. For reference, 2.4.3 was released December 2011.
It is however worth noting that postgres provides an rpm package for rhel6 for psycopg2 2.4.5 if people care about that. Personally, I don't care all that much about OS level packages. I don't now if we have any kind of official policy on them.
comment:11 by , 10 years ago
I'll defer completing this until I get some consensus about minimum supported versions.
It seems that 2.5 is an OK minimum for django.contrib.postgres
, but 2.4.5 added support for Inet arrays to the Inet
object, so that's the current minimum supported version for db.backends
without a change to the current code (at least a more informative error than the current one about new_array_type being unavailable).
People using OS packages from lts releases of distros aren't going to be keeping django up to date anyway (they'll be using whatever OS package is provided of that on their OS, probably 1.3 or earlier). I'm a fan of backwards compatibility, but I think the listed requirements are more than reasonable, considering the age of the respective packages. Obviously it's not my call though, so...
comment:12 by , 10 years ago
Has patch: | set |
---|---|
Owner: | changed from | to
New PR that implements this proposal from the mailing list thread:
- Document 2.4.3 as the minimum if you don't want to use contrib.postgres
- Document 2.5 as the minimum to use contrib.postgres
- Require 2.5+ for Django's test suite to avoid having to add conditional imports/test skipping.
comment:13 by , 10 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Personally I would be happy to simply require psycopg2 >= 2.5 - that version was released nearly 2 years ago now and I don't see any particular reason why we need to support older versions.
At the moment there is one mention of the required version, in
docs/ref/databases.txt
. This was updated to 2.0.9 earlier in this release cycle to supportUUIDField
.