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.

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

fix #96 deprecated postgres backend strings #101

Merged
merged 2 commits into from May 3, 2018

Conversation

jtdoepke
Copy link
Contributor

The database backend "django.db.backends.postgresql_psycopg2" has been deprecated
in Django 2.0 in favor of "django.db.backends.postgresql".

https://docs.djangoproject.com/en/2.0/releases/2.0/#id1

The backend "django.db.backends.postgresql_psycopg2" has been deprecated
in Django 2.0 in favor of "django.db.backends.postgresql".

https://docs.djangoproject.com/en/2.0/releases/2.0/#id1
try:
from django import VERSION as DJANGO_VERSION
except ImportError:
DJANGO_VERSION = None
Copy link
Contributor

Choose a reason for hiding this comment

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

When will this be None? More to the point, when is django.VERSION not defined?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When Django is not installed. There's no installation requirements in setup.py, and I didn't want to make the module installable but not importable.
...although adding an installation requirement for Django is probably the better way, huh?

Copy link
Contributor

Choose a reason for hiding this comment

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

Based on this comment it might be hard to find a valid lower bound.

@@ -16,7 +18,10 @@ def test_postgres_parsing(self):
url = 'postgres://uf07k1i6d8ia0v:wegauwhgeuioweg@ec2-107-21-253-135.compute-1.amazonaws.com:5431/d8r82722r2kuvn'
url = dj_database_url.parse(url)

assert url['ENGINE'] == 'django.db.backends.postgresql_psycopg2'
if DJANGO_VERSION < (2, 0):
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems to be repeated everywhere. What if we have a constant at the module level that looks like:

EXPECTED_ENGINE = 'django.db.backends.postgresql'
if DJANGO_VERSION < (2, 0):
    EXPECTED_ENGINE = 'django.db.backends.postgresql_pyscopg2'

And then we use EXPECTED_ENGINE in our assertions

@kennethreitz
Copy link
Collaborator

This library is used by many apps that require a django older than 2.0 (or even 1.8).

@kennethreitz
Copy link
Collaborator

We should detect which version of Django they are using, and act accordingly.

@kennethreitz
Copy link
Collaborator

Ideally.

@jtdoepke
Copy link
Contributor Author

jtdoepke commented May 2, 2018

Yep, it does that. Still uses the old engine for Django < 2.0.

@kennethreitz
Copy link
Collaborator

Excellent :)

@@ -6,4 +6,36 @@ python:
- 3.4
Copy link
Contributor

Choose a reason for hiding this comment

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

@kennethreitz It seems Travis isn't running the tests any longer. Can you investigate that?

Copy link
Collaborator

Choose a reason for hiding this comment

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

flipped the switch

@sigmavirus24
Copy link
Contributor

Closing & re-opening to trigger a travis build

@sigmavirus24 sigmavirus24 reopened this May 3, 2018
@sigmavirus24 sigmavirus24 merged commit 53c3d79 into jazzband:master May 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants