On Sat, Aug 22, 2009 at 1:34 PM, Andi
Albrecht<[email protected]> wrote:
>
> On Fri, Aug 21, 2009 at 1:45 PM, Russell
> Keith-Magee<[email protected]> wrote:
>>
>> On Fri, Aug 21, 2009 at 5:53 PM, Zachary
>> Voase<[email protected]> wrote:
>>>
>>> Hi Andi,
>>>
>>> On 21 Aug 2009, at 05:34, Andi Albrecht wrote:
>>>
>>>> Hi,
>>>>
>>>> I'm interested in working on #10355 "Add support for email backends."
>>>>
>>>> IMHO it's an good idea to make the email backend configurable. There
>>>> are at least two use cases I can think of. The first is to send email
>>>> with other services than SMTP, like App Engine as noted in the
>>>> ticket's description. The second is to deliver email asynchronously,
>>>> like the django-mailer application does already.
>>>
>>> I wholeheartedly agree.
>>>
>>>> The ticket currently needs a design decision, so my question is what
>>>> the actual concerns to change this are.
>>>>
>>>> I would propose the following changes. It's a very simplistic approach
>>>> that tries to keep the current API as much as possible:
>>>>
>>>> Add a new setting EMAIL_BACKEND. A string that can be resolved to a
>>>> class. Default should be the current SMTP implementation.
>>>>
>>>> Provide a base class for mail backends. A mail backend must provide
>>>> the method send_messages(email_messages) and must return the number of
>>>> messages sent to provide backward compatibility. The constructor of a
>>>> mail backend should take at least the keyword argument fail_silently
>>>> (default: False). What I'm a bit unsure about are additional
>>>> constructor arguments. Currently the SMTP backend allows in addition
>>>> host, port, username, password and use_tls. Those are very
>>>> SMTP-specific, but only username and password are used by the
>>>> mail.send_mail* APIs. It would be an agreement to allow username and
>>>> password in addition to fail_silently to not break the send_mail* API.
>>>> The SMTP backend could accept host, port and use_tls as extra keywords
>>>> again to provide backward compatibility for code that directly uses
>>>> SMTPConnection (within Django SMTPConnection is not used outside
>>>> django.core.mail). I would suggest to rename SMTPConnection to
>>>> SMTPBackend, but only if this would break too much third-party code as
>>>> SMTPConnection is mentioned in the docs.
>>>
>>> This I disagree with slightly. My main concern is the single-backend
>>> architecture; many websites will probably want to use more than one
>>> method for sending e-mail.
>>
>> I'm not sure I agree with your assertion of "many"."Some" might be
>> accurate. "Your" is probably more accurate :-)
>>
>> I've got many websites in the field, and not one of them has needed
>> anything more than trivial email handling. We've managed to get to
>> v1.1 and AppEngine support is the first time that pluggable email
>> backends have really been raised as an issue.
>>
>> This is hardly surprising. After all, email is email. You have an SMTP
>> server, you connect to it, you send your mail. AppEngine is a weird
>> case in that they provide an email-sending API rather than using SMTP,
>> but that's an artefact of the platform. Once you have one email
>> sending capability, I find it hard to believe that most people will
>> need a second.
>>
>> I don't doubt that there are applications that will require more than
>> one mail server, but I'm comfortable calling them edge cases. If you
>> have highly specialized mail requirements, then it makes sense that
>> you should have a highly specialized mail server handling.
>>
>> That said, there isn't really that much difference between the simple
>> and complex case - it's just a matter of defaults.
>>
>> Django needs to have a default Email backend, guaranteed available.
>>
>> EmailMessage.send() uses the 'default' backend - essentially just
>> calling backend.send_messages([msg])
>>
>> backend.send_messages() also exists as a direct call.
>> SMTPConnection().send_messages() is really just a shortcut for
>> instantiating and using an SMTP connection with the default settings.
>>
>> You're not compelled to use the default connection though. You could
>> instantiate multiple instances of different backends, and use them to
>> call other_backend.send_messages().
>>
>> I see this working almost exactly as the Cache backend works right
>> now. There is a base interface for caching. There are several cache
>> backends; dummy and locmem are handy for testing, but if you're
>> serious, the only one that gets used is memcached. There is a default
>> cache instantiated as a result of the CACHE_BACKEND setting. For most
>> people, this is all you will ever need. However, if you want to
>> instantiate (and use) multiple caches (e.g., if you want to get really
>> fancy about cache overflow and expiry policies), you can.
>>
>>> In addition, if mail backends only need to
>>> implement one method, why not just have EMAIL_BACKEND refer to a
>>> callable instead?
>>
>> Persistence of settings. s1 = Backend(host, port, username password)
>> can be configured once, then s1 can be reused whenever needed.
>
> There's another reason for having classes: The EmailMesage has a
> "connection" instance attribute. It's documented as "An SMTPConnection
> instance. Use this parameter if you want to use the same connection
> for multiple messages. If omitted, a new connection is created when
> send() is called."
>
> So persistence is needed here to allow multiple calls to
> EmailMessage.send() to share the same connection. I'm reading
> "connection" here as "network connection", otherwise it wouldn't make
> much sense as the creation of SMTPConnection, and possible other
> backends likewise, is pretty cheap.
>
> Actually the documentation lacks some explanation on this part - and
> therefore it took me a while to understand the connection attribute :)
> After reading the docs I've had expected that the following example
> uses a single network connection to send mails:
>
> conn = SMTPConnection()
> msg1 = EmailMessage(connection=conn).send()
> msg2 = EmailMessage(connection=conn).send()
>
> But each call to send() opens and closes a new network connection. To
> have a single network connection you'll have to do the following:
>
> conn = SMTPConnection()
> conn.open()
> msg1 = EmailMessage(connection=conn).send()
> msg2 = EmailMessage(connection=conn).send()
> conn.close()
>
> If we want to have an instance of the mail backend on module level
> similar to the backends in the cache or db module those calls to
> open() and close() become nasty as we've might get problems with
> thread-safety here (assuming that the relation between backend and
> network connection is 1:1).
>
> I would propose that backends have those open() and close() methods
> that do nothing in the base implementation. Only backends that require
> a connection on network level should overwrite those methods to manage
> network connections. I think in most cases (that is mail.send_mail*,
> backend.send_messages and EmailMessage.send without setting the
> connection instance attribute) the backend handles network connections
> (if needed) internally. The open() and close() methods are just needed
> for the example outlined above and the docs should be updated
> accordingly. (A use case for this pattern is if you want to use a
> single network connection and if you want to do things after each
> message is sent. backend.send_message([mgs1, msg2]) is not an option
> then.)
>
> Instead of having a backend instance on module level, the mail module
> should provide a get_backend(fail_silently=False) function that
> returns an backend instance. This would avoid problems with concurrent
> calls to open/close on a global backend instance.
It sounds like you've got a good handle on the problem. I can't see
anything fundamentally wrong with what you are proposing. You are
correct that the 'instance' problem doesn't really apply to the cache,
so different handling is appropriate here.
My only issue is one of nomenclature. While you will be defining an
SMTP backend and dummy backend, etc, and those classes should be
referred to as Backends, it doesn't make much sense to me to talk
about obtaining an instance of a backend or get_backend();
get_connection() would seem more appropriate terminology.
So, the public interface in django.core.mail will look a little like:
def get_connection(*args, **kwargs):
module = <resolve backend module from settings>
return module.EmailBackend(*args, **kwargs)
SMTPConnection = smtp.EmailBackend
and the get_connection call in EmailMessage, send_mail and
send_mass_mail would make a call to the get_connection utliity, rather
than instantiating SMTPConnection directly as they do now. At the end
of the day, it's actually quite a low impact change.
Yours,
Russ Magee %-)
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Django developers" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to
[email protected]
For more options, visit this group at
http://groups.google.com/group/django-developers?hl=en
-~----------~----~----~----~------~----~------~--~---
