[PATCH 3/3] parsemail: Mail error information to ADMINS when parsing fails

Finucane, Stephen stephen.finucane at intel.com
Mon Sep 28 23:53:23 AEST 2015


> On Sun, Sep 27, 2015 at 11:14:00PM +0100, Finucane, Stephen wrote:
> > > +extra_error_message = '''
> > > +== Mail
> > > +
> > > +%(mail)s
> > > +
> > > +
> > > +== Traceback
> > > +
> > > +'''
> >
> > Can you put two spaces before the below function (PEP8 thing)?
> 
> https://www.python.org/dev/peps/pep-0008/#a-foolish-consistency-is-the-
> hobgoblin-of-little-minds
> 
> In this case, I think it'd be better to leave one blank line to be
> consistent with the rest of the file and project.

True that. Precedence of File > Project > PEP8

> > > +# Send emails to settings.ADMINS when encountering errors
> > > +def setup_error_handler():
> > > +    if settings.DEBUG:
> > > +        return None
> > > +
> > > +    mail_handler = AdminEmailHandler()
> > > +    mail_handler.setLevel(logging.ERROR)
> > > +    mail_handler.setFormatter(logging.Formatter(extra_error_message))
> > > +
> > > +    logger = logging.getLogger('patchwork')
> > > +    logger.addHandler(mail_handler)
> > > +
> > > +    return logger
> > > +
> > 'setup_error_handler' is small and specific enough that it could
> > probably go into the main method, but that's not a big deal.
> 
> Not cluttering main() with details about logging that are better hidden
> behind a function wins. That's why functions exist after all?

Fair enough.

Pending that one line:

Acked-by: Stephen Finucane <stephen.finucane at intel.com>

> --
> Damien


More information about the Patchwork mailing list