API is potentially redundant

[tilgovi]

It seems to me that repoze.sendmail has an SMTP mailer and a sendmail mailer. Both implement IMailer. Why does pyramid_mailer create both and expose itself as a wrapper around both with separate API functions?

Is there a common deployment scenario where one application would want to use both methods to send mail?

It seems to me that this just creates incompatibilities where some module authors will get the mailer and call send() while others will call send_sendmail().

Would it not be better to choose, by configuration, which mailer is used, so that module authors can just call send() and the deployment can choose which is used?

[tseaver]

This package offers a tighter integration into the Pyramid ecosphere than repoze.sendmail. I can't imagine there is any need for a user who installs this package to think about repoze.sendmail at all, or try to use it directly.

[mmerickel]

FWIW I tend to agree that send, send_to_queue, send_sendmail, send_immediately and send_immediately_sendmail, and are not ideal APIs and rather this should be simply hidden behind a send that is defined at configuration-time. At least I find it difficult to see why that's an issue, especially if it accepted a **kw to override the default send.

[mmerickel]

@tseaver I'm not sure your comment is relevant.. the OP isn't bringing up repoze.sendmail but rather the silly apis on pyramid_mailer.Mailer.

[tilgovi]

@tseaver added me to the team for this repo a while back. I had been wanting to facilitate a release to get the debug mailer changes out. I'll take a look at this issue if I can, soon, but wanted to get feedback.

How would we feel about simply send, send_immediately and send_to_queue and letting sendmail use be pure configuration?

[mmerickel]

This sounds acceptable to me but even then send and send_immediately feel like all that are necessary. No?

[mmerickel]

I guess I can see send_to_queue if send is configured to send directly upon commit of the transaction which it is. I should stop this stream of consciousness.

[tilgovi]

send_to_queue works differently. A separate process is expected to flush that queue.

[mmerickel]

Sure, but isn't it still transactional? Is it really a good idea to leave it up to the caller whether the email gets send in the background or foreground? Feels like a config-time option.

[tilgovi]

I guess that depends on the intended use.

I could image wanting to process the queue mail differently. It may be bulk mail that needs to go through some tracking service that handles unsubscribe or something. It may need a different SMTP server, to ensure transactional mail doesn't end up undeliverable because your bulk mailings are getting marked as spam. On the other hand, the non-queued sends would be for transactional mail in response to direct user action.

On the other hand, if that's not the intended usage, then I agree it should be a config-time option.

[mmerickel]

Well there's no way atm to send_immediately_to_queue either.

[tilgovi]

I'm not familiar enough with the repoze mail. Is it the case that both delivery mechanisms delay until transaction commit?

[mmerickel]

Yeah the DirectMailDelivery (send and send_sendmail) and QueuedMailDelivery (send_to_queue) and are transactional whereas the other mechanisms (immediate-mode) are not.

[tilgovi]

Let's be careful with the language here, too. "Transactional" has a meaning in mailing that is opposite "bulk", in addition to the meaning of "takes effect iff the transaction commits" here. I don't think we've confused this in the thread here yet, but just pointing that out for sanity.

But, let's talk about transactions as in the transaction module. We could have send_to_queue_immediately by using repoze.sendmail.maildir directly.

Right now we seem to have two dimensions (and you rightly point out we are missing a quadrant):

Transaction vs Non-transaction Now(-ish) vs Queued

We don't have non-transactional, queued. We do have sendmail/smtp as a third dimension, but I think we all agreed that's unnecessary. But the other two dimensions might both be good.

At the least, the transaction support seems great, especially because we inherit it with little effort from the repoze package.

And queue vs non-queue feels important because the mail world has this notion of transational vs bulk mail and I can see using it for that difference. But maybe that's the wrong use case and I'm wrong and this should be pure configuration.

So, I think we can check the box for "remove the send_sendmail and send_immediately_sendmail" APIs. No consensus yet on anything else.

Does that sound right?

[mmerickel]

I'm going to try to resurrect this. Probably via cutting the current pyramid_mailer as 1.0 and then discussing a 2.0 API that standardizes on .send() with config-defined defaults.

IMO pyramid_mailer is currently a poor abstraction because people sending email still need to think about which mailer they want - how many times should this be different in different parts of your app? I'm currently thinking about the following.

class Mailer:
    def send(self, message, **kw):
        """
        Send a message.

        By default, all options are taken from the default mailer (usually
        automatically constructed from the application settings).

        :param immediate: If ``True`` the message will be sent immediately
        on its own transaction.

        :param transaction_manager: Override the default
        ``transaction_manager`` bound to the mailer. This option is mutually
        exclusive with the ``immediate`` option.

        :param queue: If ``True`` the mail will be sent to the local queue
        defined by the ``queue_path``. This will error if no ``queue_path``
        has been defined.

        :param queue_path: Override the default ``queue_path`` setting.

        :param sendmail: If ``True`` the message will be sent using the
        ``sendmail_app`` and ``sendmail_template`` settings.

        :param sendmail_app: The sendmail executable. This value is
        interpolated into the ``sendmail_template`` argument list.

        :param sendmail_template: The command-line argument list invoked.
        If you just need to change the executable then set ``sendmail_app``
        instead.

        """

This would be accompanied by the mail.default_delivery setting which would be direct by default:

mail.default_delivery = direct | queue | sendmail

In general it's possible to make everything immediate if necessary because we can wrap it in a new local transaction manager and call commit - so any delivery mechanism that is transactional can be made immediate.

Does anyone have anything to say about this idea?

[tilgovi]

:+1:

[tilgovi]

This doesn't restrict any flexibility but it means most people, most of the time, can just call send(message), which is what I think is most valuable. People are used to thinking of kwargs as secondary / if necessary / additional options, but having to decide which of five API calls to make to send a message is what I really disliked.

[jvanasco]

I generally like the idea of consolidating everything within a single send but I'd like to make a few suggestions

• Since the above would use the configured defaults, I think there should be a config option for using (default) or avoiding a transaction. I have a few apps where opting-in to making email delivery dependent on a transaction is much preferable to opting-out.
• Why would an 'immediate' message be sent within it's own transaction? Why not bypass transaction and remove it as a dependency.
• I think there should be a smtp_mailer kwarg, which would allow for a additional configured SmtpMailer instances. This would require a bit more work to track the mailer, but it looks like the same exact changes would be needed to implement the queue_path override as originally suggested.

fwiw our typical use-cases:

• a handful of apps only use pyramid_mail for reporting certain errors.
• multiple mail delivery systems may be used by a single app. some are SMTP, some are API, some are sendmail.
• sometimes everything goes to a sendmail, which will route to an internal relay that figures out how to really deliver the message based on custom headers.

It might be nice to provide for an abstract API based email delivery class in the future.