[PATCH 5/6] parsemail: Convert to a management command
Stephen Finucane
stephenfinucane at hotmail.com
Mon Sep 26 01:14:21 AEST 2016
On 23 Sep 10:06, Daniel Axtens wrote:
> Management comands allow applications to register their own actions with
> 'manage.py'. This provides some advantages, like automatically
> configuring Django (removing the need for 'django.setup' calls) and
> removing the need to set the PYTHON_PATH. The 'parsemail' script is a
> natural fit for this type of application. Migrate 'parsemail' to a
> management command.
>
> This includes some extensive work on logging configuration, as logging
> is moved from code into settings. In addition, it removes a lot of the
> customizable logging previously introduced in the parsemail command, in
> favour of modifications to the settings files.
>
> Originally-by: Stephen Finucane <stephenfinucane at hotmail.com>
> Partial-bug: #17
> Closes-bug: #15
> [dja: significant changes:
> - fix merge conflicts with my changes to parsemail.sh
> - py3 binary file email parsing
> - make stdin tests close and re-open stdin]
> Signed-off-by: Daniel Axtens <dja at axtens.net>
Reviewed-by: Stephen Finucane <stephenfinucane at hotmail.com>
> ---
>
> Stephen: the changes here are significant enough that I thought it
> might be inappropriate to keep your signed-off-by until you've had
> a chance to review them. If you're happy with them I'm more than
> happy for you to add your sign-off when you merge it.
> ---
> patchwork/bin/parsemail.py | 114 -----------------------------
> patchwork/bin/parsemail.sh | 8 +-
> patchwork/management/commands/parsemail.py | 84 +++++++++++++++++++++
> patchwork/parser.py | 12 +--
> patchwork/settings/base.py | 60 +++++++++++++--
> patchwork/tests/__init__.py | 23 ++++++
> patchwork/tests/test_management.py | 83 +++++++++++++++++++++
> patchwork/tests/test_parser.py | 4 +-
> patchwork/tests/utils.py | 2 +-
> 9 files changed, 260 insertions(+), 130 deletions(-)
> delete mode 100755 patchwork/bin/parsemail.py
> create mode 100644 patchwork/management/commands/parsemail.py
> create mode 100644 patchwork/tests/test_management.py
>
> diff --git a/patchwork/bin/parsemail.py b/patchwork/bin/parsemail.py
> deleted file mode 100755
> index b35b1f62ef9a..000000000000
> --- a/patchwork/bin/parsemail.py
> +++ /dev/null
> @@ -1,114 +0,0 @@
> -#!/usr/bin/env python
> -#
> -# Patchwork - automated patch tracking system
> -# Copyright (C) 2008 Jeremy Kerr <jk at ozlabs.org>
> -#
> -# This file is part of the Patchwork package.
> -#
> -# Patchwork is free software; you can redistribute it and/or modify
> -# it under the terms of the GNU General Public License as published by
> -# the Free Software Foundation; either version 2 of the License, or
> -# (at your option) any later version.
> -#
> -# Patchwork is distributed in the hope that it will be useful,
> -# but WITHOUT ANY WARRANTY; without even the implied warranty of
> -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> -# GNU General Public License for more details.
> -#
> -# You should have received a copy of the GNU General Public License
> -# along with Patchwork; if not, write to the Free Software
> -# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> -
> -from __future__ import absolute_import
> -
> -import argparse
> -from email import message_from_file
> -import logging
> -import sys
> -
> -import django
> -from django.conf import settings
> -from django.utils.log import AdminEmailHandler
> -
> -from patchwork.parser import parse_mail
> -
> -LOGGER = logging.getLogger(__name__)
> -
> -VERBOSITY_LEVELS = {
> - 'debug': logging.DEBUG,
> - 'info': logging.INFO,
> - 'warning': logging.WARNING,
> - 'error': logging.ERROR,
> - 'critical': logging.CRITICAL
> -}
> -
> -
> -extra_error_message = '''
> -== Mail
> -
> -%(mail)s
> -
> -
> -== Traceback
> -
> -'''
> -
> -
> -def setup_error_handler():
> - """Configure error handler.
> -
> - Ensure emails are send to settings.ADMINS when errors are
> - encountered.
> - """
> - if settings.DEBUG:
> - return
> -
> - 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
> -
> -
> -def main(args):
> - django.setup()
> - logger = setup_error_handler()
> - parser = argparse.ArgumentParser()
> -
> - def list_logging_levels():
> - """Give a summary of all available logging levels."""
> - return sorted(list(VERBOSITY_LEVELS.keys()),
> - key=lambda x: VERBOSITY_LEVELS[x])
> -
> - parser.add_argument('infile', nargs='?', type=argparse.FileType('r'),
> - default=sys.stdin, help='input mbox file (a filename '
> - 'or stdin)')
> -
> - group = parser.add_argument_group('Mail parsing configuration')
> - group.add_argument('--list-id', help='mailing list ID. If not supplied '
> - 'this will be extracted from the mail headers.')
> - group.add_argument('--verbosity', choices=list_logging_levels(),
> - help='debug level', default='info')
> -
> - args = vars(parser.parse_args())
> -
> - logging.basicConfig(level=VERBOSITY_LEVELS[args['verbosity']])
> -
> - mail = message_from_file(args['infile'])
> - try:
> - result = parse_mail(mail, args['list_id'])
> - if result:
> - return 0
> - return 1
> - except:
> - if logger:
> - logger.exception('Error when parsing incoming email', extra={
> - 'mail': mail.as_string(),
> - })
> - raise
> -
> -if __name__ == '__main__':
> - sys.exit(main(sys.argv))
> diff --git a/patchwork/bin/parsemail.sh b/patchwork/bin/parsemail.sh
> index 241f18c6bbc9..b6e11457bbc0 100755
> --- a/patchwork/bin/parsemail.sh
> +++ b/patchwork/bin/parsemail.sh
> @@ -31,6 +31,12 @@ if [ -z $DJANGO_SETTINGS_MODULE ]; then
> fi
>
> PYTHONPATH="$PATCHWORK_BASE":"$PATCHWORK_BASE/lib/python:$PYTHONPATH" \
> - $PW_PYTHON "$PATCHWORK_BASE/patchwork/bin/parsemail.py" $@
> + $PW_PYTHON "$PATCHWORK_BASE/manage.py" parsemail $@
>
> +# NOTE(stephenfin): We must return 0 here. When parsemail is used as a
> +# delivery command from a mail server like postfix (as it is intended
> +# to be), a non-zero exit code will cause a bounce message to be
> +# returned to the user. We don't want to do that for a parse error, so
> +# always return 0. For more information, refer to
> +# https://patchwork.ozlabs.org/patch/602248/
> exit 0
> diff --git a/patchwork/management/commands/parsemail.py b/patchwork/management/commands/parsemail.py
> new file mode 100644
> index 000000000000..d454ce544644
> --- /dev/null
> +++ b/patchwork/management/commands/parsemail.py
> @@ -0,0 +1,84 @@
> +# Patchwork - automated patch tracking system
> +# Copyright (C) 2016 Stephen Finucane <stephenfinucane at hotmail.com>
> +#
> +# This file is part of the Patchwork package.
> +#
> +# Patchwork is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# Patchwork is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with Patchwork; if not, write to the Free Software
> +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> +
> +import argparse
> +import email
> +import logging
> +from optparse import make_option
> +import sys
> +
> +import django
> +from django.core.management import base
> +from django.utils import six
> +
> +from patchwork.parser import parse_mail
> +
> +logger = logging.getLogger(__name__)
> +
> +
> +class Command(base.BaseCommand):
> + help = 'Parse an mbox file and store any patch/comment found.'
> +
> + if django.VERSION < (1, 8):
> + args = '<infile>'
> + option_list = base.BaseCommand.option_list + (
> + make_option(
> + '--list-id',
> + help='mailing list ID. If not supplied, this will be '
> + 'extracted from the mail headers.'),
> + )
> + else:
> + def add_arguments(self, parser):
> + parser.add_argument(
> + 'infile',
> + nargs='?',
> + type=str,
> + default=None,
> + help='input mbox file (a filename or stdin)')
> + parser.add_argument(
> + '--list-id',
> + help='mailing list ID. If not supplied, this will be '
> + 'extracted from the mail headers.')
> +
> + def handle(self, *args, **options):
> + infile = args[0] if args else options['infile']
> +
> + if infile:
> + logger.info('Parsing mail loaded by filename')
> + if six.PY3:
> + with open(infile, 'rb') as file_:
> + mail = email.message_from_binary_file(file_)
> + else:
> + with open(infile) as file_:
> + mail = email.message_from_file(file_)
> + else:
> + logger.info('Parsing mail loaded from stdin')
> + if six.PY3:
> + mail = email.message_from_binary_file(sys.stdin.buffer)
Ah...good catch.
> + else:
> + mail = email.message_from_file(sys.stdin)
> + try:
> + result = parse_mail(mail, options['list_id'])
> + if result:
> + sys.exit(0)
> + logger.warning('Failed to parse mail')
> + sys.exit(1)
> + except Exception:
> + logger.exception('Error when parsing incoming email',
> + extra={'mail': mail.as_string()})
> diff --git a/patchwork/parser.py b/patchwork/parser.py
> index 1b4cab1eb1a8..eefdb6f94156 100644
> --- a/patchwork/parser.py
> +++ b/patchwork/parser.py
> @@ -42,7 +42,7 @@ _hunk_re = re.compile(r'^\@\@ -\d+(?:,(\d+))? \+\d+(?:,(\d+))? \@\@')
> _filename_re = re.compile(r'^(---|\+\+\+) (\S+)')
> list_id_headers = ['List-ID', 'X-Mailing-List', 'X-list']
>
> -LOGGER = logging.getLogger(__name__)
> +logger = logging.getLogger(__name__)
>
>
> def normalise_space(str):
> @@ -654,7 +654,7 @@ def parse_mail(mail, list_id=None):
>
> hint = mail.get('X-Patchwork-Hint', '').lower()
> if hint == 'ignore':
> - LOGGER.debug("Ignoring email due to 'ignore' hint")
> + logger.debug("Ignoring email due to 'ignore' hint")
> return
>
> if list_id:
> @@ -663,7 +663,7 @@ def parse_mail(mail, list_id=None):
> project = find_project_by_header(mail)
>
> if project is None:
> - LOGGER.error('Failed to find a project for email')
> + logger.error('Failed to find a project for email')
> return
>
> # parse content
> @@ -706,7 +706,7 @@ def parse_mail(mail, list_id=None):
> delegate=delegate,
> state=find_state(mail))
> patch.save()
> - LOGGER.debug('Patch saved')
> + logger.debug('Patch saved')
>
> return patch
> elif x == 0: # (potential) cover letters
> @@ -738,7 +738,7 @@ def parse_mail(mail, list_id=None):
> submitter=author,
> content=message)
> cover_letter.save()
> - LOGGER.debug('Cover letter saved')
> + logger.debug('Cover letter saved')
>
> return cover_letter
>
> @@ -759,7 +759,7 @@ def parse_mail(mail, list_id=None):
> submitter=author,
> content=message)
> comment.save()
> - LOGGER.debug('Comment saved')
> + logger.debug('Comment saved')
>
> return comment
>
> diff --git a/patchwork/settings/base.py b/patchwork/settings/base.py
> index b78ed4b0fd64..a32710f1d02b 100644
> --- a/patchwork/settings/base.py
> +++ b/patchwork/settings/base.py
> @@ -125,6 +125,8 @@ STATICFILES_DIRS = [
> # Third-party application settings
> #
>
> +# rest_framework
> +
> try:
> # django rest framework isn't a standard package in most distros, so
> # don't make it compulsory
> @@ -136,17 +138,65 @@ try:
> except ImportError:
> pass
>
> -#
> -# Third-party application settings
> -#
> -
> -# rest_framework
>
> REST_FRAMEWORK = {
> 'DEFAULT_VERSIONING_CLASS': 'rest_framework.versioning.NamespaceVersioning'
> }
>
> #
> +# Logging settings
> +#
> +
> +LOGGING = {
> + 'version': 1,
> + 'disable_existing_loggers': False,
> + 'formatters': {
> + 'email': {
> + 'format': '== Mail\n\n%(mail)s\n\n== Traceback\n',
> + },
> + },
> + 'filters': {
> + 'require_debug_false': {
> + '()': 'django.utils.log.RequireDebugFalse',
> + },
> + 'require_debug_true': {
> + '()': 'django.utils.log.RequireDebugTrue',
> + },
> + },
> + 'handlers': {
> + 'console': {
> + 'level': 'DEBUG',
> + 'filters': ['require_debug_true'],
> + 'class': 'logging.StreamHandler',
> + },
> + 'mail_admins': {
> + 'level': 'ERROR',
> + 'filters': ['require_debug_false'],
> + 'class': 'django.utils.log.AdminEmailHandler',
> + 'formatter': 'email',
> + 'include_html': True,
> + },
> + },
> + 'loggers': {
> + 'django': {
> + 'handlers': ['console'],
> + 'level': 'INFO',
> + 'propagate': True,
> + },
> + 'patchwork.parser': {
> + 'handlers': ['console'],
> + 'level': 'DEBUG',
> + 'propagate': False,
> + },
> + 'patchwork.management.commands': {
> + 'handlers': ['console', 'mail_admins'],
> + 'level': 'INFO',
> + 'propagate': True,
> + },
> + },
> +}
> +
> +#
> # Patchwork settings
> #
>
> diff --git a/patchwork/tests/__init__.py b/patchwork/tests/__init__.py
> index e69de29bb2d1..f6b9104de392 100644
> --- a/patchwork/tests/__init__.py
> +++ b/patchwork/tests/__init__.py
> @@ -0,0 +1,23 @@
> +# Patchwork - automated patch tracking system
> +# Copyright (C) 2016 Stephen Finucane <stephenfinucane at hotmail.com>
> +#
> +# This file is part of the Patchwork package.
> +#
> +# Patchwork is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# Patchwork is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with Patchwork; if not, write to the Free Software
> +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> +
> +import os
> +
> +TEST_MAIL_DIR = os.path.join(os.path.dirname(__file__), 'mail')
> +TEST_PATCH_DIR = os.path.join(os.path.dirname(__file__), 'patches')
> diff --git a/patchwork/tests/test_management.py b/patchwork/tests/test_management.py
> new file mode 100644
> index 000000000000..5f97aa76866f
> --- /dev/null
> +++ b/patchwork/tests/test_management.py
> @@ -0,0 +1,83 @@
> +# Patchwork - automated patch tracking system
> +# Copyright (C) 2016 Stephen Finucane <stephenfinucane at hotmail.com>
> +#
> +# This file is part of the Patchwork package.
> +#
> +# Patchwork is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# Patchwork is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with Patchwork; if not, write to the Free Software
> +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> +
> +import os
> +import sys
> +
> +from django.core.management import call_command
> +from django.test import TestCase
> +
> +from patchwork import models
> +from patchwork.tests import TEST_MAIL_DIR
> +from patchwork.tests import utils
> +
> +
> +class ParsemailTest(TestCase):
> +
> + def test_invalid_path(self):
> + # this can raise IOError, CommandError, or FileNotFoundError,
> + # depending of the versions of Python and Django used. Just
> + # catch a generic exception
> + with self.assertRaises(Exception):
> + call_command('parsemail', infile='xyz123random')
> +
> + def test_missing_project_path(self):
> + path = os.path.join(TEST_MAIL_DIR, '0001-git-pull-request.mbox')
> + with self.assertRaises(SystemExit) as exc:
> + call_command('parsemail', infile=path)
> +
> + self.assertEqual(exc.exception.code, 1)
> +
> + def test_missing_project_stdin(self):
> + path = os.path.join(TEST_MAIL_DIR, '0001-git-pull-request.mbox')
> + sys.stdin.close()
> + sys.stdin = open(path)
> + with self.assertRaises(SystemExit) as exc:
> + call_command('parsemail', infile=None)
> +
> + self.assertEqual(exc.exception.code, 1)
> +
> + def test_valid_path(self):
> + project = utils.create_project()
> + utils.create_state()
> +
> + path = os.path.join(TEST_MAIL_DIR, '0001-git-pull-request.mbox')
> + with self.assertRaises(SystemExit) as exc:
> + call_command('parsemail', infile=path, list_id=project.listid)
> +
> + self.assertEqual(exc.exception.code, 0)
> +
> + count = models.Patch.objects.filter(project=project.id).count()
> + self.assertEqual(count, 1)
> +
> + def test_valid_stdin(self):
> + project = utils.create_project()
> + utils.create_state()
> +
> + path = os.path.join(TEST_MAIL_DIR, '0001-git-pull-request.mbox')
> + sys.stdin.close()
> + sys.stdin = open(path)
> + with self.assertRaises(SystemExit) as exc:
> + call_command('parsemail', infile=None,
> + list_id=project.listid)
> +
> + self.assertEqual(exc.exception.code, 0)
> +
> + count = models.Patch.objects.filter(project=project.id).count()
> + self.assertEqual(count, 1)
> diff --git a/patchwork/tests/test_parser.py b/patchwork/tests/test_parser.py
> index 17e0e7ae3468..c0e3bb61725a 100644
> --- a/patchwork/tests/test_parser.py
> +++ b/patchwork/tests/test_parser.py
> @@ -39,6 +39,7 @@ from patchwork.parser import parse_mail as _parse_mail
> from patchwork.parser import parse_pull_request
> from patchwork.parser import parse_series_marker
> from patchwork.parser import split_prefixes
> +from patchwork.tests import TEST_MAIL_DIR
> from patchwork.tests.utils import create_project
> from patchwork.tests.utils import create_state
> from patchwork.tests.utils import create_user
> @@ -46,9 +47,6 @@ from patchwork.tests.utils import read_patch
> from patchwork.tests.utils import SAMPLE_DIFF
>
>
> -TEST_MAIL_DIR = os.path.join(os.path.dirname(__file__), 'mail')
> -
> -
> def read_mail(filename, project=None):
> """Read a mail from a file."""
> file_path = os.path.join(TEST_MAIL_DIR, filename)
> diff --git a/patchwork/tests/utils.py b/patchwork/tests/utils.py
> index 29455d2bdc2e..23b0c87cda56 100644
> --- a/patchwork/tests/utils.py
> +++ b/patchwork/tests/utils.py
> @@ -32,6 +32,7 @@ from patchwork.models import Patch
> from patchwork.models import Person
> from patchwork.models import Project
> from patchwork.models import State
> +from patchwork.tests import TEST_PATCH_DIR
>
> SAMPLE_DIFF = """--- /dev/null 2011-01-01 00:00:00.000000000 +0800
> +++ a 2011-01-01 00:00:00.000000000 +0800
> @@ -39,7 +40,6 @@ SAMPLE_DIFF = """--- /dev/null 2011-01-01 00:00:00.000000000 +0800
> +a
> """
> SAMPLE_CONTENT = 'Hello, world.'
> -TEST_PATCH_DIR = os.path.join(os.path.dirname(__file__), 'patches')
>
>
> def read_patch(filename, encoding=None):
> --
> 2.7.4
>
More information about the Patchwork
mailing list