[PATCH v6 1/2] parsemail: Convert to a management command
Daniel Axtens
dja at axtens.net
Mon Sep 19 20:53:36 AEST 2016
Hi Stephen,
I know we're already at a pretty ridiculous number of revisions for
this, but...
> + parser.add_argument(
> + 'infile',
> + nargs='?',
> + type=argparse.FileType('r'),
> + default=sys.stdin,
> + help='input mbox file (a filename or stdin)')
I'm working on fixing the bug Thomas encountered earlier this
month, and this use of FileType is causing me some issues in terms of
dealing with encoding in a cross-platform way.
Python 3's FileType allows you to specify an encoding and what to do
with errors (strict/ignore/replace etc). Python 2's FileType has no such
parameters.
Looking at some other people's experience with FileType - e.g
http://stackoverflow.com/questions/18862836/how-to-open-file-using-argparse#comment34787630_18863046
I wonder if we might be better off replacing the file type with a file
name and dealing with errors opening the file ourselves. Then we have
full control of all these aspects.
Happy to have a crack at this myself if you'd prefer.
Regards,
Daniel
> + 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']
> +
> + # Attempt to parse the path if provided, and fallback to stdin if not
> + if infile and isinstance(infile, six.string_types): # Django < 1.8
> + logger.info('Parsing mail loaded by filename')
> + with open(infile, 'r+') as file_:
> + mail = message_from_file(file_)
> + else:
> + if infile == sys.stdin:
> + logger.info('Parsing mail loaded from stdin')
> + else: # Djano >= 1.8
> + logger.info('Parsing mail loaded by filename')
> + mail = message_from_file(infile)
> + 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 1805df8..51de997 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):
> @@ -599,7 +599,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:
> @@ -608,7 +608,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
> @@ -651,7 +651,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
> @@ -683,7 +683,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
>
> @@ -704,7 +704,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 b78ed4b..a32710f 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 e69de29..f6b9104 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 0000000..6cd21e2
> --- /dev/null
> +++ b/patchwork/tests/test_management.py
> @@ -0,0 +1,80 @@
> +# 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
> +
> +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')
> + with open(path) as file_:
> + with self.assertRaises(SystemExit) as exc:
> + call_command('parsemail', infile=file_)
> +
> + 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')
> + with open(path) as file_:
> + with self.assertRaises(SystemExit) as exc:
> + call_command('parsemail', infile=file_,
> + 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 7b5c71b..d3f6954 100644
> --- a/patchwork/tests/test_parser.py
> +++ b/patchwork/tests/test_parser.py
> @@ -38,6 +38,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
> @@ -45,9 +46,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 29455d2..23b0c87 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
>
> _______________________________________________
> Patchwork mailing list
> Patchwork at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 859 bytes
Desc: not available
URL: <http://lists.ozlabs.org/pipermail/patchwork/attachments/20160919/3b431aae/attachment.sig>
More information about the Patchwork
mailing list