Django

Code

Ticket #6835 (closed: fixed)

Opened 9 months ago

Last modified 9 months ago

django.core.mail.SMTPConnection.open() method performs an unnecessary call to socket.getfqdn()

Reported by: George Murdocca <gmurdocca@gmail.com> Assigned to: PhiR
Milestone: Component: django.core.mail
Version: SVN Keywords: local_hostname, smtplib, CachedDnsName, SMTPConnection
Cc: gmurdocca@gmail.com Triage Stage: Ready for checkin
Has patch: 1 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 0

Description

Summary

This is an optimization fix as opposed to a bug. The django.core.mail.SMTPConnection.open() method performs an extraneous call to socket.getfqdn() which affects performance. The mail.py module has a class defined called CachedDnsName() whos get_fqdn() method addresses this issue by calling socket.getfqdn(), and caches the result, thus removing the need to call it again later. The fix is extremely trivial to implement. Here are the details:

Detailed Description

On line 122 of the mail.py module, an smtplib.SMTP() object is instantiated:

self.connection = smtplib.SMTP(self.host, self.port)

The constructor of smtplib.SMTP() accepts three arguments, namely host='', port=0, local_hostname=None. However, local_hostname is omitted in mail.py. When omitted, socket.getfqdn() is called, and its value is assigned to the missing argument. This call is extraneous and slow, because: * socket.getfqdn() is slow to evaluate (as mentioned in the comment at line 28 of mail.py) * The result of socket.getfqdn() already done once and cached by django.core.mail.CachedDnsName.get_fqdn(), and shoul dbe used instead.

The Fix

Change line 122 of mail.py to:

self.connection = smtplib.SMTP(self.host, self.port, local_hostname=DNS_NAME.get_fqdn())

The Diff

--- mail.py     2008-03-02 12:45:20.000000000 +1100
+++ mailjrj.py  2008-03-20 14:04:07.000000000 +1100
@@ -119,7 +119,7 @@
             # Nothing to do if the connection is already open.
             return False
         try:
-            self.connection = smtplib.SMTP(self.host, self.port)
+            self.connection = smtplib.SMTP(self.host, self.port, local_hostname=DNS_NAME.get_fqdn())
             if self.use_tls:
                 self.connection.ehlo()
                 self.connection.starttls()

Cheers, George Murdocca & Phil Wright.

Attachments

6835.diff (0.6 kB) - added by PhiR on 03/20/08 18:17:31.
proper patch against SVN

Change History

03/20/08 18:17:31 changed by PhiR

  • attachment 6835.diff added.

proper patch against SVN

03/20/08 18:19:00 changed by PhiR

  • needs_better_patch changed.
  • needs_tests changed.
  • owner changed from nobody to PhiR.
  • needs_docs changed.
  • has_patch set to 1.
  • stage changed from Unreviewed to Ready for checkin.

applies to SVN, and successfully tested on my app.

03/21/08 16:08:40 changed by gwilson

  • status changed from new to closed.
  • resolution set to fixed.

(In [7348]) Fixed #6835 -- Use cached FQDN when creating smtplib.SMTP() connection to avoid a lengthy socket.getfqdn() call, thanks George Murdocca and PhiR.


Add/Change #6835 (django.core.mail.SMTPConnection.open() method performs an unnecessary call to socket.getfqdn())




Change Properties
Action