Code

Opened 22 months ago

Closed 6 weeks ago

#18985 closed Bug (fixed)

DeprecationWarning no longer loud by default in Python 2.7+

Reported by: dstufft Owned by: nobody
Component: Core (Other) Version: 1.5
Severity: Release blocker Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

Since Python 2.7 (and 3.2) Python no longer emits output for DeprecationWarnings. This is due to the desire to hide these errors from end users. The Django release documentation still references the fact that these errors will be loud by default (and relies on it to help people migrate to new features).

I propose modifying the documentation of the release process to include a note about how to restore that output, as well as modifying the default manage.py to re-emit DeprecationWarning.

Attachments (3)

18985-2.diff (1.2 KB) - added by claudep 21 months ago.
Reactivate DeprecationWarning on 2.7+
18985-3.diff (3.5 KB) - added by claudep 16 months ago.
Setting py.warnings handler as soon as possible for tests (patch for 1.5)
18985-4.diff (6.2 KB) - added by claudep 16 months ago.
Added test for module-level warnings, and let -W option take priority

Download all attachments as: .zip

Change History (32)

comment:1 Changed 22 months ago by dstufft

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Pull request included here: https://github.com/django/django/pull/385

comment:2 Changed 22 months ago by dstufft

There has apparently been discussion in the past about wether Django should follow Python's lead and should teach using -Wd, or if it should do something along the lines as what I've proposed.

The original change to hide DeprecationWarning by default in Python was made to hide that warning from end users of Python applications who might be using a newer version of Python than what the application was developed on. Seeing these warnings has caused confusion amongst these end users. In order to be more end user friendly Python decided to silence by default. However in Django's case there is typically not an end user in the same vein that the change was made for that might get confused by the DeprecationWarning.

Additionally because the change was made in the manage.py in the defaule startproject that means that for anyone using WSGI (I'm going to assume the vast majority of production users are using WSGI) that production will be unaffected. Additionally it will only affect new projects created after the change went it. Finally since it is a part of manage.py and not a part of Django proper if someone does want this behavior reverted it's a simple edit of a local file.

comment:3 Changed 22 months ago by dstufft

After discussion with ptone on IRC three points have been raised that a change to make DeprecationWarning loud again needs to hit these points: early, configurable, universal. My PR as it stands is early and configurable, but it's not universal (It doesn't touch WSGI, but more importantly it doesn't affect anyone who currently has a project without them manually adding it.)

The options as it stands to re-emit the warning are:

  1. Add a filter to the default manage.py. This happens before any other code happens so it is the earliest entry point. It also is easily configurable as it is included in the users project code. However this is the least universal as it will only affect new projects created in 1.5 unless developers of existing projects add the same code to their manage.py
  1. Add a filter someplace early on in Django. This will be less early than the manage.py solution, but it should be early enough to catch all the warnings. This is also universal, any users of Django 1.5 will get this change without needing to make any changes to their project. This one falls down at the configurable stage, without either providing another settings value (or possibly keying it off of DEBUG?) there would not be a way for a developer to silence this warning without directly modifying Django. While it's true that the majority of projects do not have end users that would be affected by the DeprecationWarning display it's likely that there are at least some of them and this change would remove the developers ability to silence them.
  1. Add a filter someplace early on in Django, but turn on logging.captureWarnings so that all the warnings get redirected to a named logger called py.warnings with a level of WARNING. This would likely need to have the default LOGGING configuration changed to output py.warnings to STDOUT (or STDERR). This solution would be early and configurable, and it would also be universal as long as the project in question has a logging configuration that would result in messages sent to py.warnings with level WARNING to be emitted. If, for example, they have a root logger configured this would be sufficient.
Last edited 22 months ago by dstufft (previous) (diff)

comment:4 Changed 22 months ago by ptone

Looks like #18993 could make option 3 easier to choose

Changed 21 months ago by claudep

Reactivate DeprecationWarning on 2.7+

comment:5 Changed 21 months ago by claudep

  • Has patch set
  • Triage Stage changed from Unreviewed to Accepted

The above patch is a combination of 2 and 3, as logging.captureWarnings by itself does not change the visibility of DeprecationWarning. One drawback of this solution is that DeprecationWarnings are not visible before settings are configured.

comment:6 Changed 21 months ago by ptone

  • Patch needs improvement set

So there was discussion back in Sept on IRC that didn't make it onto this ticket.

The consensus there was to aim for the best possible balance of flexible but universal

Because we are adding the simplefilter in Django conf (meaning we clobber any silencing simplefilter in customized manage.py) - and the deprecations could be annoying in production logs - they should be conditional on DEBUG=True

If we do that - then the logging.captureWarning parts are superfluous- we could note in the docs, that if you want to silence warnings in debug - you can use that, but to capture warnings and route them to a console logger as a default just becomes redundant.

To handle your point about DeprecationWarnings not being visible until settings are configured - we can address that for new projects by ADDITIONALLY putting the simplefilter in the default manage.py as was done in the original pull request here.

I don't think that the python logging implementation is so braindead that it adds the same filter twice - but geez - I guess we should check.

That way 1.5 projects get the earliest possible warnings, while upgraded 1.4 projects (with the original manage.py) or 1.5 projects with custom manage.py replacements at least get runtime deprecation warnings while in debug mode.

comment:7 Changed 20 months ago by ptone

So going another round on this - if we wanted to make Django "special" and buck what core-python decided WRT DeprecationWarnings being ignored, I would be suggesting putting the simplefilter in django/init as the place to "change the runtime". However there is a strong argument against making Django special in the Python ecosphere. Instead if we consider that when DEBUG true - we are running in a "special" mode, and so do special things, that would be more reasonable.
I'm going to step back from the idea of putting the simplefilter in manage.py as something that is harder to disable - and generally not going to gain all that much in terms of early deprecations.

The main deprecations missed will be those directly related to settings themselves, but the only way around that is to deviate farther toward the path of being "special"

A remaining question - and it is minor, is whether or not to have the simplefilter entwined with the logging setup.

The patch from Claude is very reasonable, the only other way to get to a similar state would be something like:

https://github.com/ptone/django/compare/ticket/18985-loud-deprecation

The only thing in favor of that is if you were to do something custom with the console handler, and didn't want that to be applied to warnings - you'd have to de-couple them. But at that point it would be just as easy to also change the py.warnings logger.

So I suppose I'm slightly more in favor of going through logging.

comment:8 Changed 20 months ago by aaugustin

I don't think we should diverge from Python, no matter how we feel about that decision.

IMO the reasonable course of action is to fix the docs and teach -Wd.

comment:9 Changed 20 months ago by russellm

@aaugustin - I agree with the broad principle that we shouldn't diverge from Python, but in this case, it seems to undermine our broader strategy. We specifically chose PendingDeprecationWarning and DeprecationWarning as a 2 phase mechanism for slowly introducing change in a way that couldn't be easily ignored; Python core policy seems to be undermining our intentions in this case.

Another option: Can we introduce a new warning subclass that *isn't* silenced by default? Reading the 2.7 release notes, it seems clear to me that the intention from Python core is that DeprecationWarning is for a different purpose to what we're using it; in which case, it would make sense to introduce a new warning type that serves *our* purposes.

comment:10 Changed 20 months ago by ptone

If I have to pick between siding with what is in the best interest of Django Users, or Python core, I'm going to bias slightly toward the users. I had also considered using a different warning class - but I think we are actually using DeprecationWarning exactly as intended - and really the decision is the same as changing the filters, just getting there via another route. It seems pretty clear that Django is a developer tool - not end user software. The end users are web users and aren't exposed to warnings.

http://docs.python.org/dev/whatsnew/2.7.html#the-future-for-python-2-x

Reenabling warning output is suggested as an option in the above docs - so I'm not sure why it is considered anti-python (Alex to this position on IRC) to choose to display warnings in a context where it makes sense (a Django development environment).

Another option is to just enable the deprecation warning output inside the runserver command - as we make clear this is a dev server.

comment:11 Changed 20 months ago by aaugustin

OK. Making them verbose when DEBUG = True sounds like a good compromise.

comment:12 Changed 20 months ago by claudep

Just in case this wan't clear, the latest patch I uploaded on this ticket only outputs anything when DEBUG is True (by default), as the console logging handler is filtered by require_debug_true. Of course, this is all configurable by a custom logging config.

comment:13 Changed 20 months ago by Preston Holmes <preston@…>

  • Resolution set to fixed
  • Status changed from new to closed

In 0d49fdb573d44794cc78c6af4761cc79c5330315:

[1.5.x] Fixed #18985 -- made DeprecationWarnings loud

Capture warnings in Python >= 2.7 and route through
console handler, which is subject to DEBUG==True

Thanks to dstufft for the idea, and claudep for initial patch

comment:14 Changed 20 months ago by Preston Holmes <preston@…>

In 44046e8a38225067d4d0feac35367eeae133446a:

Fixed #18985 -- made DeprecationWarnings loud

Capture warnings in Python >= 2.7 and route through
console handler, which is subject to DEBUG==True

Thanks to dstufft for the idea, and claudep for initial patch

comment:15 Changed 20 months ago by Preston Holmes <preston@…>

In c9b1e6ceb018ee52cd7764ae1f4f88b353c17fb4:

Fixed #18985 -- made DeprecationWarnings loud

Capture warnings in Python >= 2.7 and route through
console handler, which is subject to DEBUG==True

Thanks to dstufft for the idea, and claudep for initial patch

comment:16 Changed 18 months ago by Preston Holmes <preston@…>

In cfa70d0c94a43d94e3ee48db87f2b88c29a862e1:

Fixed #19546 - ensure that deprecation warnings are shown during tests

refs #18985

comment:17 Changed 18 months ago by Preston Holmes <preston@…>

In af8e858c1596623ffcccedc57433a009455bc314:

[1.5.x] Fixed #19546 - ensure that deprecation warnings are shown during tests

refs #18985

comment:18 Changed 17 months ago by Preston Holmes <preston@…>

In af8e858c1596623ffcccedc57433a009455bc314:

[1.5.x] Fixed #19546 - ensure that deprecation warnings are shown during tests

refs #18985

comment:19 Changed 16 months ago by aaugustin

  • Resolution fixed deleted
  • Severity changed from Normal to Release blocker
  • Status changed from closed to new

In some circumstances, this change silences deprecation warnings even under -Wd — I hope you appreciate the irony ;)

I'm not sure how this happens exactly, but it affects warnings emitted at compile time (ie. at the module level).

This emits a pending deprecation warning:

tests % PYTHONPATH=.. python2.6 -Wd runtests.py --settings=test_sqlite comments

This doesn't:

tests % PYTHONPATH=.. python2.7 -Wd runtests.py --settings=test_sqlite comments

Reverting this change fixes the second case.


Currently, the 1.5 branch contains several module-level deprecation warnings:

django % grep -r -A2 '^warnings\.warn' django
django/conf/urls/defaults.py:warnings.warn("django.conf.urls.defaults is deprecated; use django.conf.urls instead",
django/conf/urls/defaults.py-              DeprecationWarning)
django/conf/urls/defaults.py-
--
django/contrib/databrowse/__init__.py:warnings.warn("The Databrowse contrib app is deprecated", DeprecationWarning)
--
django/contrib/localflavor/__init__.py:warnings.warn("django.contrib.localflavor is deprecated. Use the separate django-localflavor-* packages instead.", DeprecationWarning)
--
django/contrib/localflavor/id/id_choices.py:warnings.warn(
django/contrib/localflavor/id/id_choices.py-    'There have been recent changes to the ID localflavor. See the release notes for details',
django/contrib/localflavor/id/id_choices.py-    RuntimeWarning
--
django/contrib/localflavor/uk/forms.py:warnings.warn(
django/contrib/localflavor/uk/forms.py-    'The "UK" prefix for United Kingdom has been deprecated in favour of the '
django/contrib/localflavor/uk/forms.py-    'GB code. Please use the new GB-prefixed names.', DeprecationWarning)
--
django/contrib/localflavor/uk/uk_regions.py:warnings.warn(
django/contrib/localflavor/uk/uk_regions.py-    'The "UK" prefix for United Kingdom has been deprecated in favour of the '
django/contrib/localflavor/uk/uk_regions.py-    'GB code. Please use the new GB-prefixed names.', DeprecationWarning)
--
django/utils/copycompat.py:warnings.warn("django.utils.copycompat is deprecated; use the native copy module instead",
django/utils/copycompat.py-              DeprecationWarning)
django/utils/copycompat.py-
--
django/utils/hashcompat.py:warnings.warn("django.utils.hashcompat is deprecated; use hashlib instead",
django/utils/hashcompat.py-              DeprecationWarning)
django/utils/hashcompat.py-
--
django/utils/simplejson.py:warnings.warn("django.utils.simplejson is deprecated; use json instead.",
django/utils/simplejson.py-              PendingDeprecationWarning)
django/utils/simplejson.py-

These warnings are currently silent in 1.5, even under -Wd, and some of these features have already been removed in 1.6!

That's also why #20065 wasn't detected earlier — Jacob and I spent a bit of time and didn't think of running tests under 2.6.

comment:20 Changed 16 months ago by jacob

  • Version changed from master to 1.5

comment:21 Changed 16 months ago by claudep

  • Version changed from 1.5 to master

Here's my recent findings about this issue:

Warnings are redirected to the logging system, which is configured by default to use the 'console' handler. By default, that handler is filtering out events when DEBUG is False. That is wanted for deployed Django apps, except for tests. That was the reason for commit [cfa70d0c94a43d9].
Unfortunately, all warnings emitted from the time they are redirected to the logging system until DjangoTestSuiteRunner.run_tests force to reenable warnings display are lost (module-level warnings), because DEBUG is initially False.

So one solution is to reenable the non-filtering StreamHandler for 'py.warnings' sooner in the process, in setup() of runtests.py (before get_apps() which triggers the module warnings).

comment:22 Changed 16 months ago by claudep

  • Version changed from master to 1.5

Sorry, didn't mean to reset version.

Changed 16 months ago by claudep

Setting py.warnings handler as soon as possible for tests (patch for 1.5)

Changed 16 months ago by claudep

Added test for module-level warnings, and let -W option take priority

comment:23 follow-up: Changed 16 months ago by ptone

Claude,

I've given this a pretty thorough review, and it looks good. I'm wondering if you forgot a git clean somewhere, as the tests in the diff were still written to the location in tests/regressiontests

I've staged your patch, with only one small addition to a comment, just to clarify that the warnings may not be even captured at this point, didn't figure it was worth another check to sys.warnoptions, but that could be another way to clarify those lines (since they are so far from the conf/init code)

https://github.com/ptone/django/compare/18985-fix#L3R87

comment:24 in reply to: ↑ 23 Changed 16 months ago by claudep

Replying to ptone:

... I'm wondering if you forgot a git clean somewhere, as the tests in the diff were still written to the location in tests/regressiontests

My patch was based on the 1.5 branch.

I've staged your patch, with only one small addition to a comment, just to clarify that the warnings may not be even captured at this point, didn't figure it was worth another check to sys.warnoptions, but that could be another way to clarify those lines (since they are so far from the conf/__init__ code)

https://github.com/ptone/django/compare/18985-fix#L3R87

Thanks for reviewing!

comment:25 Changed 16 months ago by Preston Holmes <preston@…>

  • Resolution set to fixed
  • Status changed from new to closed

In e79b857a07905340556f781a7d63016236b21c61:

Fixed #18985 -- ensure module level deprecations are displayed

Also don't compete with -W CLI option.

Thanks to Aymeric Augustin for the catch, and Claude Paroz for the patch.

comment:26 Changed 16 months ago by jacob

  • Resolution set to fixed
  • Status changed from new to closed

Fixed in [f6989e5].

comment:27 Changed 16 months ago by Jacob Kaplan-Moss <jacob@…>

In 572a300e56bc0192a9c82565a1d39b96f18bebee:

[1.5.x] Fixed #18985 -- ensure module level deprecations are displayed

Also don't compete with -W CLI option.

Thanks to Aymeric Augustin for the catch, and Claude Paroz for the patch.

Backport of e79b857a07905340556f781a7d63016236b21c61 from master.

comment:28 Changed 6 weeks ago by anonymous

  • Resolution fixed deleted
  • Status changed from closed to new

We are using python library to run emulator tool for software.
Sometimes, if we launch execution with python2.7, it will generate RAMP dump error and csv file is not generated properly(instead of adding content in new line, it is appended to same line with missing some data).

Whereas, if i run same script with python2.5, csv is generated properly with deprecation warning.

Anyhow i dont worry about warning, python2.5 worked for me.

But why python2.7 is generating RAMP dump error?

comment:29 Changed 6 weeks ago by timo

  • Resolution set to fixed
  • Status changed from new to closed

Please use our support channels to get help.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.