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?

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.

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()))