Opened 13 years ago

Closed 13 years ago

Last modified 13 years ago

#16736 closed Bug (fixed)

AdminEmailHandler ignores user args to log message.

Reported by: saverio.trioni@… 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)

adminemail_getmessage_patch.2.diff (2.3 KB ) - added by Saverio Trioni 13 years ago.
adminemail_getmessage_patch.3.diff (1.5 KB ) - added by Saverio Trioni 13 years ago.
Sorry, there was a typo due to excessive vimming…
adminemail_getmessage_patch.diff (2.3 KB ) - added by Saverio Trioni 13 years ago.
This is the final patch, ignore the other ones.
16736.adminemailhandler-formatted-subject.diff (2.8 KB ) - added by Julien Phalip 13 years ago.

Download all attachments as: .zip

Change History (13)

comment:1 by Saverio Trioni, 13 years ago

Has patch: set
Needs tests: set

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.

comment:2 by Saverio Trioni, 13 years ago

Owner: changed from nobody to Saverio Trioni
Status: newassigned

comment:3 by Julien Phalip, 13 years ago

Component: UncategorizedCore (Other)
Triage Stage: UnreviewedAccepted

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 Saverio Trioni, 13 years ago

comment:4 by Saverio Trioni, 13 years ago

New patch, with regression test.

by Saverio Trioni, 13 years ago

Sorry, there was a typo due to excessive vimming...

by Saverio Trioni, 13 years ago

This is the final patch, ignore the other ones.

comment:5 by Julien Phalip, 13 years ago

Triage Stage: AcceptedReady for checkin

Looks good to me. I've just tweaked the test a little.

comment:6 by Saverio Trioni, 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?

in reply to:  6 comment:7 by Julien Phalip, 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:8 by Julien Phalip, 13 years ago

Resolution: fixed
Status: assignedclosed

In [16715]:

Fixed #16736 -- Enabled the merging of user-supplied arguments to format the error emails' subject in AdminEmailHandler.

comment:9 by Julien Phalip, 13 years ago

Thank you strioni for the report and patch, and really sorry that I forgot to mention you in the commit message.

Note: See TracTickets for help on using tickets.
Back to Top