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 DeprecationWarnings from Django 1.9 #2392

Closed

Conversation

mx-moth
Copy link
Contributor

@mx-moth mx-moth commented Mar 23, 2016

The output of ./runtests.py was so full of DeprecationWarnings, RuntimeWarnings, and so on that it was near useless. This PR aims to clean the output up some what.

The console output is full of deprecation warnings produced by Wagtail that need fixing. It is also full of deprecation warnings from third party packages such as django-taggit. I've made the test suite only print deprecation warnings produced by Wagtail by default, so the warnings are easier to deal with. There is a --full-warnings flag that can be passed to ./runtests.py to enable all deprecation warnings again, if we want to try and fix warnings in third party modules.

Wagtail was still using some of its own deprecated internal methods, despite having a documented alternative available. I've fixed these warnings as well.

There were two places still using field.related instead of field.rel, and rel.to over rel.model. These have been fixed. Django 1.9 renamed field.rel to field.remote_model, deprecating the old field.rel, so this will need to happen again in the future.

Model._meta.get_fields() should be used, instead of the old Model._meta.get_all_related_objects(). Django has instructions on migrating this code.

All management commands now use argparse/add_arguments() over optparse/options_list.

Some tests used naive datetimes, which was causing needless RuntimeWarnings.

Elasticsearch was complaining about not verifying SSL certificates for HTTPS connections. This change is mildly backwards incompatible, as SSL certificates are now verified by default. People will have to change their configurations if they are using Elasticsearch over HTTPS, with an invalid SSL certificate, through the automatic URLS parameter. A release note has been added, with further discussion in the relevant commit.

Some tests were missing from __future__ import unicode_literals, which was causing a RuntimeWarning from unidecode via django-taggit. unicode_literals have been enabled in the relevant tests. I will create a different PR which adds unicode_literals to all files in the repo, as this is a larger and slightly unrelated change.

The end result of all this is that the output of ./runtests.py on Django 1.8 is now free of warnings!

@mx-moth mx-moth force-pushed the refactor/django-deprecation-warnings branch from 5dc9ded to 88c64de Compare March 23, 2016 06:28
@SalahAdDin
Copy link
Contributor

👍

@mx-moth mx-moth force-pushed the refactor/django-deprecation-warnings branch 3 times, most recently from e04b4e5 to 358c654 Compare March 24, 2016 01:14
@mx-moth
Copy link
Contributor Author

mx-moth commented Mar 24, 2016

Alright, this is no longer WIP. All DeprecationWarnings and PendingDeprecationWarnings have been fixed for Django 1.8; which fixes all DeprecationWarnings for Django 1.9. I can not fix the PendingDeprecationWarnings for Django 1.9 easily, as fixing those breaks Django 1.8 compatibility. As 1.10 is not due to be released for some time, dropping support for 1.8 is not sensible just yet. We can live with the warnings in 1.9.

By default, only warnings for Wagtail are shown. I can see a sensible argument for showing warnings generated by Willow, django-modelcluster, and other packages written for Wagtail. Should I enable these warnings by default? All warnings can be switched on using ./runtests.py --deprecation=all, so these warnings can still be found.

I've added some more commits, descriptions are in the ticket description above.

@mx-moth mx-moth changed the title [WIP] Fix deprecation warnings from Django 1.9 Fix DeprecationWarnings from Django 1.9 Mar 24, 2016
@mx-moth
Copy link
Contributor Author

mx-moth commented Mar 24, 2016

I've also fixed many of the deprecation warnings for django-taggit, to ensure Wagtail continues to work with future versions of Django: jazzband/django-taggit#385. Using the code from this PR, the only DeprecationWarnings raised in the tests comes from django-rest-framework


Elasticsearch would not be configured to verify SSL certificates for HTTPS URLs. This has been changed so that SSL certificates are verified for HTTPS connections by default.

If you need the old behaviour back, where SSL certificates are not verified for your HTTPS connection, you can configure the Elasticsearch backend with the HOSTS option, like so:
Copy link
Contributor

Choose a reason for hiding this comment

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

HOSTS should be backticked like HOSTS

@kaedroho
Copy link
Contributor

Looks good to me!

The test output was extremely noisy with deprecation warnings. Half of
these warnings were caused by external modules like django-taggit, and
could be ignored. The warnings caused by Wagtail got lost in the noise
though, rendering the test output useless for finding warnings. By
default, the tests now only print deprecation warnings if raised by
Wagtail.

Use `./runtests.py --deprecation=all` if you want all the warnings to be
printed. This is useful when we want to fix third party packages before
they break from being too outdated.

Use `./runtests.py --deprecation=pending` if you want the default
behaviour.

Use `./runtests.py --deprecation=imminent` if you only want imminent
DeprecationWarnings, ignoring PendingDeprecationWarnings.

Use `./runtests.py --deprecation=none` if you want to ignore all
deprecation warnings.
Some code was using methods from Wagtail, even though those methods were
deprecated with alternatives provided. Those alternatives are now used
instead.
Using naive datetimes was raising RuntimeWarnings
Making developers opt out of extra security is better than making them
opt in, especially when they may not be aware of the security they are
missing out on.
These tests sent some Python 2 `str`s to unidecode via taggit, which
raised a RuntimeWarning. These strings should be unicode, and are
unicode when they come from Django outside of the tests.

unicode_literals should be added to all Python files to ensure
consistent handling of strings across Python versions, but that is a
larger and more controversial change.
@mx-moth mx-moth force-pushed the refactor/django-deprecation-warnings branch from 358c654 to 77b0060 Compare March 24, 2016 22:42
@mx-moth
Copy link
Contributor Author

mx-moth commented Mar 24, 2016

I've fixed the issues you mentioned, and rebased on master

mx-moth added a commit to neon-jungle/wagtail that referenced this pull request Mar 28, 2016
Drone will now check that

    from __future__ absolute_import, unicode_literals

is part of every Python source file, to ensure a consistent experience
across all versions of Python.

See wagtail#2392 for an instance where missing `unicode_literals` was causing
problems.
@tomdyson tomdyson modified the milestones: 1.4.2, 1.5 Mar 30, 2016
@kaedroho
Copy link
Contributor

I've merged all except 59599d4 into master at b3aa292 +parents

I left 59599d4 out for now as it prevents extra options being passed through to dj test. For example the --keepdb option no longer works.

I've cheery-picked the following commits into the stable/1.4.x branch:

e003140 Use add_arguments over deprecated option_list
904441d Use _meta.get_fields() over deprecated _meta.get_all_related_objects()
90bcabd Use field.rel over field.related, rel.model over rel.to
c92ed90 Stop using methods deprecated in Wagtail

@mx-moth
Copy link
Contributor Author

mx-moth commented Mar 31, 2016

Thanks for the merge.

You can still pass through extra arguments to dj test, by placing them after a --, such as:

$ ./runtests.py --deprecation=all -- --keepdb

@mx-moth
Copy link
Contributor Author

mx-moth commented Mar 31, 2016

I'll close this one and make another PR for that commit, to keep this discussion and history clean and separate.

@mx-moth mx-moth closed this Mar 31, 2016
mx-moth added a commit to neon-jungle/wagtail that referenced this pull request Mar 31, 2016
Drone will now check that

    from __future__ absolute_import, unicode_literals

is part of every Python source file, to ensure a consistent experience
across all versions of Python.

See wagtail#2392 for an instance where missing `unicode_literals` was causing
problems.
m1kola pushed a commit that referenced this pull request Apr 3, 2016
Drone will now check that

    from __future__ absolute_import, unicode_literals

is part of every Python source file, to ensure a consistent experience
across all versions of Python.

See #2392 for an instance where missing `unicode_literals` was causing
problems.

Add missing absolute_import, unicode_literals to all files

Explicitly ensure strings are of the correct types

Now that unicode_literals is in every file, some things that used to
be py2 `str`s were now `unicode` instead. This caused issues with
generated class / function names, which must be `str` in all versions of
Python. This means bytes in py2, and unicode in py3. A test also checked
for the incorrect type of SafeString. HTML content should always be
unicode, so this has been fixed.
loicteixeira pushed a commit to springload/wagtail that referenced this pull request Apr 3, 2016
Drone will now check that

    from __future__ absolute_import, unicode_literals

is part of every Python source file, to ensure a consistent experience
across all versions of Python.

See wagtail#2392 for an instance where missing `unicode_literals` was causing
problems.

Add missing absolute_import, unicode_literals to all files

Explicitly ensure strings are of the correct types

Now that unicode_literals is in every file, some things that used to
be py2 `str`s were now `unicode` instead. This caused issues with
generated class / function names, which must be `str` in all versions of
Python. This means bytes in py2, and unicode in py3. A test also checked
for the incorrect type of SafeString. HTML content should always be
unicode, so this has been fixed.
jordij pushed a commit to springload/wagtail that referenced this pull request Apr 7, 2016
Drone will now check that

    from __future__ absolute_import, unicode_literals

is part of every Python source file, to ensure a consistent experience
across all versions of Python.

See wagtail#2392 for an instance where missing `unicode_literals` was causing
problems.

Add missing absolute_import, unicode_literals to all files

Explicitly ensure strings are of the correct types

Now that unicode_literals is in every file, some things that used to
be py2 `str`s were now `unicode` instead. This caused issues with
generated class / function names, which must be `str` in all versions of
Python. This means bytes in py2, and unicode in py3. A test also checked
for the incorrect type of SafeString. HTML content should always be
unicode, so this has been fixed.
JoshBarr pushed a commit to springload/wagtail that referenced this pull request Apr 27, 2016
Drone will now check that

    from __future__ absolute_import, unicode_literals

is part of every Python source file, to ensure a consistent experience
across all versions of Python.

See wagtail#2392 for an instance where missing `unicode_literals` was causing
problems.

Add missing absolute_import, unicode_literals to all files

Explicitly ensure strings are of the correct types

Now that unicode_literals is in every file, some things that used to
be py2 `str`s were now `unicode` instead. This caused issues with
generated class / function names, which must be `str` in all versions of
Python. This means bytes in py2, and unicode in py3. A test also checked
for the incorrect type of SafeString. HTML content should always be
unicode, so this has been fixed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants