Prevent conflicts with e-mail backends where send_messages() return lists

[Aramgutang]

I assume you've noticed that 3rd party Django mail backends have poor compliance with Django's spec for send_messages() to return the number of e-mails that were sent.

I see that backends that return unexpected things like None (like django-post-office does) are already handled by the code.

However, there are also popular backends like django-celery-email that return a different kind of unexpected value: a list of Celery AsyncResult objects, which can then be used to monitor the sending progress, if desired.

It appears that this incompatibility has been pointed out to the developers, and they appear a bit ambivalent about their intent to fix it. Kinda understandable, since there are probably users out that are reliant on this non-standard behaviour, and they test for it in their tests.

So since it's unlikely that it will be fixed on their end, it's probably justified to add handling for yet another kind of non-standard behaviour. Since the length of a received list is likely to correspond to the number of e-mails that were sent, I propose checking if the returned object has a length, and using that.

I should add that the impact of this incompatibility isn't actually that bad when django-post-office is also used in the project. What happens then is that the e-mails get sent, but their status gets marked as "failed" and the "can only concatenate list (not "int") to list" error message is placed in the email log. Without it, the TypeError would end up propagating, possibly resulting in a 500, though the e-mails should still get sent.

[tobiasmcnulty]

🚀 Thanks for the suggestion! This looks harmless to me, though a comment about the reason for the extra complexity might be handy in the future.

Barring no objections from other Cakti I'll merge this on Monday.

[jaap3]

Imho this is not for django-email-bandit to solve. Subclassing CeleryEmailBackend and "fixing" send_messages is a much neater solution and doesn't burden this project with a kludgy workaround. e.g.:

class CompatCeleryEmailBackend(CeleryEmailBackend):
    def send_messages(self, email_messages):
        return len(super().send_messages(email_messages))

class HijackCeleryEmailBackend(HijackBackendMixin, CompatCeleryEmailBackend):
    pass

Then configure your project to use the fixed backend instead:

EMAIL_BACKEND = 'my_project.email_backends.HijackCeleryEmailBackend'

[edit] Fixed the example

[Aramgutang]

@jaap3 I see where you're coming from. My reasoning for why it's worth addressing the issue here is threefold:

• it's not going to get addressed where it ideally should be (it's django-celery-email's fault, they should be the ones to fix it, but alas)
• there is already special handling of some non-compliant return types (i.e. None) in the bandit code, so why not add another special case (there are very unlikely to be any more)
• the issue is really non-obvious, and people will find it hard to notice and debug, so adding the special case contributes to the good in the world by reducing developers' wasted time and frustration

I agree that your proposal for subclassing the backend should work, and I'd be happy to point people towards that solution if it was officially documented somewhere. But I just don't think there's an intuitive place to put that documentation where it's likely to reliably be seen by the developers who need to see it. Maybe if there was some mechanism to alert developers about the conflict when both libraries are added to a project. Though I suppose that if this pull request is rejected, my next step would be to ask the celery-email project to put a warning in their documentation.

[jaap3]

Hijacking Arbitrary Backends would be the right place to document this.

Or you could introduce another backend using the example I gave (would need some docs as well).

[tobiasmcnulty]

Ah, good call @jaap3. I agree the subclassing approach is a cleaner solution. I like the idea of documenting this, perhaps using django-celery-email as an example for how to work around backends that aren't strictly Django-compatible.