Opened 8 years ago

Closed 8 years ago

#27830 closed Bug (fixed)

Use distutils.version.LooseVersion instead of custom version parsing

Reported by: NotSqrt Owned by: ChillarAnand
Component: Core (Other) Version: 1.10
Severity: Normal Keywords:
Cc: anand21nanda@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

Hi !

As discussed with Tim Graham in https://groups.google.com/forum/#!topic/django-users/BM3d679AsyA

django/db/backends/postgresql_psycopg2/base.py:psycopg2_version can't handle versions like 2.7b1 or 2.7b2.dev0(the current git versions), because it only returns (2, ), which is lower than the minimal requirement of (2, 4, 5).

A possibility is to use :

  from distutils.version import LooseVersion
  LooseVersion(psycopg2.__version__.split(' ', 1)[0]) >= LooseVersion('2.4.5')

Another possibility is packaging, the reference implementation of PEP-440, which is a dependency of setuptools (previously included in its vendor libs), so very probably installed everywhere !

While searching for "version" in the django code base, there appears to be multiple regex used to parse version strings : mysql, postgresql, gdal, geos.
This could probably be more DRY, and correct.

I've seen things like if geos_version_info()['version'] < '3.3.0':, which might fail at some point.

Thanks !

Change History (8)

comment:1 by Tim Graham, 8 years ago

Component: UncategorizedCore (Other)
Triage Stage: UnreviewedAccepted
Type: UncategorizedBug

comment:2 by ChillarAnand, 8 years ago

Owner: changed from nobody to ChillarAnand
Status: newassigned

comment:3 by ChillarAnand, 8 years ago

Cc: anand21nanda@… added
Has patch: set

Fixed in PR

comment:4 by Tim Graham, 8 years ago

Patch needs improvement: set

Additional tests are needed.

comment:5 by ChillarAnand, 8 years ago

Added additional test cases.

comment:6 by Tim Graham, 8 years ago

I'm doubtful that packaging is always available. I created a Python 3.6.1 virtualenv and can import setuptools ('36.0.1') but not packaging.

comment:7 by ChillarAnand, 8 years ago

Good catch. Replaced it with distutils.version.LooseVersion.

comment:8 by Tim Graham <timograham@…>, 8 years ago

Resolution: fixed
Status: assignedclosed

In 08bda82c:

Fixed #27830 -- Used distutils.version.LooseVersion for version parsing.

Note: See TracTickets for help on using tickets.
Back to Top