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
No warnings for Django 1.8 #1752
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great cleanup - my only gripe is with the confusing comments :-)
# Django >= 1.8 | ||
from django.apps import apps | ||
get_models = apps.get_models | ||
del apps | ||
except ImportError: | ||
# Django < 1.9 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't get the logic of these comments. The first branch is for Django >= 1.8
, the exception is for Django < 1.9
. So both branches include Django 1.8?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1.8 is the first version where the new method works, and the last method where the old method works :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, that clears things up, thanks! 👍
from django.db.models import get_models | ||
|
||
try: | ||
# Django >= 1.8 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same question about the version "requirements" given in these comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same reason as the previous case: 1.8 is the first version where the new method works, and the last method where the old method works. In 1.7 I think there is a warning to this effect.
IIRC we used to use the old method also on 1.8 but I switched the order so that we always prefer the new method whenever possible.
try: | ||
return connection.ops.value_to_db_datetime(value) # <= 1.8 | ||
except AttributeError: | ||
return connection.ops.adapt_datetimefield_value(value) # >= 1.9 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interestingly enough, this appears to be a no-op for the postgresql backend in Django 1.9. This function just returns its input unchanged :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heh
This PR gets rid of the remaining warnings for running NAV on Django 1.8, in code that we control.