From patchwork Fri Sep 23 00:06:16 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Axtens X-Patchwork-Id: 673757 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.ozlabs.org (lists.ozlabs.org [103.22.144.68]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3sgDGK0z4Jz9ry7 for ; Fri, 23 Sep 2016 10:08:09 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=axtens.net header.i=@axtens.net header.b=d6i6FJXx; dkim-atps=neutral Received: from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) by lists.ozlabs.org (Postfix) with ESMTP id 3sgDGJ6xLCzDsn7 for ; Fri, 23 Sep 2016 10:08:08 +1000 (AEST) Authentication-Results: lists.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=axtens.net header.i=@axtens.net header.b=d6i6FJXx; dkim-atps=neutral X-Original-To: patchwork@lists.ozlabs.org Delivered-To: patchwork@lists.ozlabs.org Received: from mail-pf0-x241.google.com (mail-pf0-x241.google.com [IPv6:2607:f8b0:400e:c00::241]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 3sgDF74w3BzDsnJ for ; Fri, 23 Sep 2016 10:07:07 +1000 (AEST) Authentication-Results: lists.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=axtens.net header.i=@axtens.net header.b=d6i6FJXx; dkim-atps=neutral Received: by mail-pf0-x241.google.com with SMTP id 21so4418886pfy.1 for ; Thu, 22 Sep 2016 17:07:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=axtens.net; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=YjQRRLXh57DzMecV7yTUDRZa6ow1mckLCQxiNO4q3po=; b=d6i6FJXxIBw21t52X88K0vyEd50y9pRnQS6+wJ1brR6BHo5FLV2avYRWZMl1oXFUXS eqe+Z+QdpsPx3RchcJTJ99165SByKZHaIkmsVfHqBKS5lU/ha91frqmXkYYlI8Yp73SN hlsYePNDWNcZwpcXfFCutV6vCdE+OA3KT7sEE= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=YjQRRLXh57DzMecV7yTUDRZa6ow1mckLCQxiNO4q3po=; b=YFibBT8grVhVaDP8bmOomGlunTDWRH40ftjZ6BAG5+cgSQGJtsz3ijvV+gKI54wyCB K+l88MNBrJSOdapsyjggTAduoWlWc2l/ZE0KjuUrUmmMCxYL/EhGFDGFHCRoOr8LXBLV zAvG9OdtfJYeLlsqsU4V8poJ9NCwbWQd1r8afhA7MOQeMo79mCPp45rXu5wnMyXHN62V RloROWSytRRXJXV3X2OAwiFfpiYhWnf09JY4I2MiJJyCFuVv2FPwRDvGub4oSZ5105k/ KEPaWsdRo1oDsqroG5YCUCqDvgJzjoP+exA25Aw7eYtiFC/ArpldBSp3HnNxPfqZfCb3 51KA== X-Gm-Message-State: AE9vXwMf36TrpXJipIkF894SPrtX9uyoDpR8rSEoMnarTOqwkwgv/aiIrWZEgdju47a0yw== X-Received: by 10.98.60.68 with SMTP id j65mr7734675pfa.55.1474589226021; Thu, 22 Sep 2016 17:07:06 -0700 (PDT) Received: from possimpible.au.ibm.com (2001-44b8-111f-5b01-0d8c-f1c6-62e0-d4b5.static.ipv6.internode.on.net. [2001:44b8:111f:5b01:d8c:f1c6:62e0:d4b5]) by smtp.gmail.com with ESMTPSA id r29sm1077731pfd.37.2016.09.22.17.07.02 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Thu, 22 Sep 2016 17:07:05 -0700 (PDT) From: Daniel Axtens To: patchwork@lists.ozlabs.org Subject: [PATCH 5/6] parsemail: Convert to a management command Date: Fri, 23 Sep 2016 10:06:16 +1000 Message-Id: <1474589177-10328-6-git-send-email-dja@axtens.net> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1474589177-10328-1-git-send-email-dja@axtens.net> References: <1474589177-10328-1-git-send-email-dja@axtens.net> X-BeenThere: patchwork@lists.ozlabs.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Patchwork development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: andrew.donnellan@au1.ibm.com MIME-Version: 1.0 Errors-To: patchwork-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org Sender: "Patchwork" 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 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 Reviewed-by: Stephen Finucane --- 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 -# -# 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 +# +# 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 = '' + 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) + 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 +# +# 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 +# +# 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):