#16288: Enabling 'django.request' logger when DEBUG is True
---------------------------------------+------------------------------
Reporter: mattbennett | Owner: nobody
Type: New feature | Status: new
Milestone: | Component: Core (Other)
Version: SVN | Severity: Normal
Resolution: | Keywords: logging
Triage Stage: Accepted | Has patch: 1
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
---------------------------------------+------------------------------
Comment (by carljm):
Replying to [comment:4 russellm]:
> Broadly speaking, this sounds ok; but where are you suggesting that we
put this shim? Off the top of my head, I can think of three options:
> 1. As part of the logging startup code. This would be fairly noisy, but
would be guaranteed to be executed
> 2. As a baked-in filtering step on the admin email logging handler.
This would effectively be duplicating the behavior of the proposed filter,
but raising a PendingDeprecationWarning as it made use of the shim. The
downside is that you won't get the PendingDeprecationWarning unless you
actually raise an error.
> 3. Same as (2), but do it in the 500 handler instead of the admin email
logging handler. This has the same problems as (2), but addresses the
problem at the source.
I had assumed (1). I think this is actually addressing the problem nearest
the source, as the problem is the default LOGGING config has changed and
they may want to update theirs. I'm not sure why it would be noisy; it
means the warning would only occur once, at settings-loading time, rather
than repeatedly on every error (and wouldn't require an error to occur),
which seems much preferable to me. And it doesn't require that we
duplicate any logic.
> > (Also, I'm wondering if this patch ought to come with a test verifying
the new behavior, that django.request loggings calls are fired in
DEBUG=True, too. We don't currently seem to have any direct tests for the
logging calls in Django, at least that I can find.)
>
> No, we don't -- mostly because the logging code is vanilla usage of
Python standard logging. All a test would verify is that the appropriate
logging calls exist where they should. There are some loose and indirect
tests of the admin email handler in regressiontests/views/tests/debug.py
and regressiontests/views/views.py, but these are really validating that
error emails contain (or don't contain) the information they should (or
shouldn't). Given that we're making the logic around the email handler
more complex, it might be worth adding some tests to make sure error
emails are sent at all.
In a way, we're making Django's logic less complex, since the logging call
now occurs regardless of DEBUG, and we're making increased use of "vanilla
Python logging" with a filter object in the default config. I don't think
we really need to test that stdlib logging works as documented. If we had
a general policy of testing that logging calls exist at certain places in
the Django codebase, then we'd want to update those tests to reflect that
this call now occurs regardless of DEBUG. But since we don't...
I think the back-compat shim is the trickiest bit of the whole proposal
and probably warrants a test or three, even though those tests would come
out again when the deprecation period runs out.
--
Ticket URL: <https://code.djangoproject.com/ticket/16288#comment:5>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/django-updates?hl=en.
