Abstracting celery tasks with class based implementations

Question

Background

I want to send emails with a rendered template (django template, that is) but I also want to be able to control the QuerySets, and context provided.

The goal is to make a task whose run method is the same (because sending an email isn't the interesting bit) for each implementation but allow the user to provide data and templates as a configuration for what the run method should feed into the template and mailer.

First pass

The first approach was to define a task for accepting serializable data with rules about how to grab objects from the database - if there was only one value then it was taken as a plain value and passed along to the template context. This lead to having tasks that required a lot of knowledge of how this stuff worked so I decided a level of abstraction was appropriate for keeping the code more concise and readable.

Full Code

# -*- coding: utf-8 -*-
from __future__ import unicode_literals
import logging
from django.core.mail import EmailMultiAlternatives as EmailMulti
from django.template.loader import render_to_string
from project.celery import app
logger = logging.getLogger(__name__)
class TemplateEmailTask(app.Task):
 '''
 Abstract base task for sending an email with one or more templates.
 Implementors should define
 1. get_context_data, a method like Django Views that should provide a
 dictionary for passing to the template renderer,
 2. a name class attribute to uniquely identify this task to Celery, and
 3. a list of template names to use in rendering the email.
 An example implementation would be:
 class MyEmail(TemplateEmailTask):
 name = 'python.module.path.to.tasks.my_email'
 template_names = []
 def get_context_data(self, **kwargs):
 return {
 today: timezone.now()
 }
 It is totally allowed to make database calls in get_context_data (that's
 why it was built this way) to provide data to the template.
 Note: it is not necessary to do anything else to get the task to register
 as long as the module has been imported somewhere during initialization of
 the Django system. If necessary, import the task module in the django app's
 apps.py file in the ready method for the AppConfig subclass.
 '''
 abstract = True
 template_names = []
 def get_template_names(self):
 '''
 Make sure we have some template names defined, i.e. the list isn't
 empty. Complain to the user if we dont.
 '''
 if len(self.template_names) == 0:
 raise ValueError(
 'No template names defined in {cls}.'
 ' You either have to define the template_names'
 ' class attribute or override #get_template_names'.format(
 cls=self.__class__.__name__))
 return self.template_names
 def get_context_data(self, **kwargs):
 '''
 Provide a method to override when defining subclasses.
 '''
 raise NotImplementedError()
 @staticmethod
 def get_suffix(template_name):
 '''
 Split the incoming file name and grab the file extension.
 '''
 return template_name.split('.', -1)[-1]
 def render(self, template_name, context):
 '''
 Get a string representation of the template + context.
 '''
 suffix = self.get_suffix(template_name)
 try:
 rendered = render_to_string(template_name, context)
 except Exception as e:
 logger.error(e.message)
 raise
 else:
 return suffix, rendered
 @staticmethod
 def make_message(subject=None,
 text_body=None,
 from_email='',
 to_emails=[],
 html_content=None,
 reply_to=''):
 '''
 Email Multi Alternatives factory.
 '''
 msg = EmailMulti(subject,
 text_body,
 from_email,
 to_emails,
 reply_to=reply_to)
 if html_content:
 msg.attach_alternative(html_content, 'text/html')
 return msg
 def run(self, subject, *args, **kwargs):
 '''
 This method is meant to not be overriden, it performs that actual call
 when a task is received on the consumer.
 '''
 context = self.get_context_data(**kwargs)
 rendered = {suffix: rendered
 for suffix, rendered in
 [self.render(template_name, context)
 for template_name in self.get_template_names()]}
 mail = self.make_message(
 subject=subject,
 text_body=rendered.get('txt', ''),
 from_email='',
 to_emails=[],
 reply_to='',
 html_content=rendered.get('html', None))
 return mail.send()

Actual Questions

1. Is there a more reasonable way to do this? should I use the inheritance mechanism to accomplish this or should I instead use some other approach which is more declarative in the context of celery
2. Is the documentation/ method names of this class sufficient to convey meaning? does this interface represent what is most necessary to allow this pattern to exist in other places?
3. Any other notes about the module's structure, control flow, pythonic-ness, documentation, really any review you might wish to offer. Does this look/read like something that someone with import this tattooed on them would write?

• Kevin Brown-Silva: Side note: I recognize that this is solving a similar problem is a new way, but django-mailviews exists to template email messages and send them using the standard Django mail system. You may be interested in it, if only for a reference.
• theWanderer4865: Thanks! Haven't heard of that one yet! That is what this code sort of evolved into as I wrote it - glad to see that it wasn't totally nuts.

Answer 1

You can improve the code a bit:

• if len(self.template_names) == 0 is better written as if not self.template_names

• logger.error can be replaced by logger.exception as it will wrap the message of the current exception automatically:

  try:
   rendered = render_to_string(template_name, context)
  except Exception:
   logger.exception('render failed on %s\n', template_name)
   raise
  

• you can simplify the iteration when creating rendered:

  rendered = {
   self.get_suffix(template_name): self.render(template_name, context)
   for template_name in self.get_template_names()
  }
  

  this means you need to simplify self.render so that it only render the template:

  def render(self, template_name, context):
   try:
   return render_to_string(template_name, context)
   except Exception:
   logger.exception('render failed on %s\n', template_name)
   raise
  

  but it can't be a bad thing to separate concerns.

• theWanderer4865: Thank you for the feedback! Thanks on the exception method - forgot about that. I'll play with the render method a bit more because you're right - it knows a little too much at this point.

Answer 2

Few things here:

First

PEP257 Dosstrings should use tripple-double-quotes but not tripple-single-quotes as you do.

Second

def make_message(subject=None,
 text_body=None,
 from_email='',
 to_emails=[],
 html_content=None,
 reply_to=''):

To avoid some issues, you better use immutable defaults so to_emails should be either tuple or None.

Third

rendered = {suffix: rendered
 for suffix, rendered in
 [self.render(template_name, context)
 for template_name in self.get_template_names()]}

This compression can be a bit prettified

render = partial(self.render, context=context)
rendered = dict(map(render, self.get_template_names()))

• theWanderer4865: Thank you for your feedback! I tend to prefer comprehensions where the encourage readability but you're right - this one isn't that readable. Also, good point on the tuple - I'll have to double check the django docs on acceptable types for that param.