#16736 closed Bug (fixed)
AdminEmailHandler ignores user args to log message.
Reported by: | Owned by: | Saverio Trioni | |
---|---|---|---|
Component: | Core (Other) | Version: | 1.3 |
Severity: | Normal | Keywords: | logging |
Cc: | Triage Stage: | Ready for checkin | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | yes | UI/UX: | no |
Description
When using AdminEmailHandler to log own code, the mail subject and content has the raw LogRecord msg attribute instead of the LogRecord.getMessage() merged string.
This appears when using the standard log API
foo, bar = 'hello', 'world' logger.error("The configuration is %s %s", foo, bar)
The email subject should be "The configuration is hello world" (barring configured prefixes), but is "The configuration is %s %s". The same happens to the email content.
This can be solved using record.getMessage()
instead of record.msg
to get the message.
Attachments (4)
Change History (13)
comment:1 by , 13 years ago
Has patch: | set |
---|---|
Needs tests: | set |
comment:2 by , 13 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:3 by , 13 years ago
Component: | Uncategorized → Core (Other) |
---|---|
Triage Stage: | Unreviewed → Accepted |
This makes sense. For testing, maybe look at customising the send_log
method in [1]
. See also [2]
for how to test emails.
[1]
source:django/trunk/tests/regressiontests/views/views.py?rev=16606#L139
[2]
https://docs.djangoproject.com/en/dev/topics/testing/#email-services
by , 13 years ago
Attachment: | adminemail_getmessage_patch.2.diff added |
---|
by , 13 years ago
Attachment: | adminemail_getmessage_patch.3.diff added |
---|
Sorry, there was a typo due to excessive vimming...
by , 13 years ago
Attachment: | adminemail_getmessage_patch.diff added |
---|
This is the final patch, ignore the other ones.
comment:5 by , 13 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
Looks good to me. I've just tweaked the test a little.
by , 13 years ago
Attachment: | 16736.adminemailhandler-formatted-subject.diff added |
---|
follow-up: 7 comment:6 by , 13 years ago
Needs tests: | unset |
---|
Seems good to me, I didn't know about the override_settings decorator.
Just one point, should the EMAIL_SUBJECT_PREFIX setting be overridden too, in case the test is run with alternate settings module?
comment:7 by , 13 years ago
Replying to strioni:
Just one point, should the EMAIL_SUBJECT_PREFIX setting be overridden too, in case the test is run with alternate settings module?
Yes that's a good idea. Feel free to update the patch, otherwise it can be done later on commit.
comment:9 by , 13 years ago
Thank you strioni for the report and patch, and really sorry that I forgot to mention you in the commit message.
I find it difficult to write a regression test for this, as the
AdminEmailHandler.emit
is sending an email directly. Anyway a patch is attached.