[PATCH] parsemail: Clarify exit codes

[Daniel Axtens]

Daniel Axtens <dja at axtens.net> writes:

> jk reports that the patchwork error codes are really unhelpful for
> correct integration with an MDA. In particular they make sorting out
> failures into a separate queue very difficult. Make this better and
> clearer.
>
> Update the comment for parse_mail regarding return values and exceptions
> to line up with how the function actually works and how we use the
> results.
>

jk applied this to OzLabs and used it to verify the bugs fixed in
stable/2.0, so I am considering that to be a pretty solid test, and
am merging this to master and stable/2.0.

Regards,
Daniel

> Reported-by: Jeremy Kerr <jk at ozlabs.org>
> Fixes: #171
> Signed-off-by: Daniel Axtens <dja at axtens.net>
> ---
>  patchwork/management/commands/parsemail.py | 16 ++++++++++++----
>  patchwork/parser.py                        |  9 ++++++++-
>  2 files changed, 20 insertions(+), 5 deletions(-)
>
> diff --git a/patchwork/management/commands/parsemail.py b/patchwork/management/commands/parsemail.py
> index 52ec8bc56899..1ed12ae886cf 100644
> --- a/patchwork/management/commands/parsemail.py
> +++ b/patchwork/management/commands/parsemail.py
> @@ -77,12 +77,20 @@ class Command(base.BaseCommand):
>              logger.warning("Broken email ignored")
>              return
>  
> +        # it's important to get exit codes correct here. The key is to allow
> +        # proper separation of real errors vs expected 'failures'.
> +        #
> +        # patch/comment parsed:        0
> +        # no parseable content found:  0
> +        # db integrity/other db error: 1 (this will mean dups are logged, which
> +        #                                 might get annoying - we'll see)
> +        # broken email (ValueError):   1 (this could also be noisy, if an issue
> +        #                                 we could use a different return code)
>          try:
>              result = parse_mail(mail, options['list_id'])
> -            if result:
> -                sys.exit(0)
> -            logger.warning('Failed to parse mail')
> -            sys.exit(1)
> +            if result is None:
> +                logger.warning('Nothing added to database')
>          except Exception:
>              logger.exception('Error when parsing incoming email',
>                               extra={'mail': mail.as_string()})
> +            sys.exit(1)
> diff --git a/patchwork/parser.py b/patchwork/parser.py
> index c1f40a577fa3..019ca20bb159 100644
> --- a/patchwork/parser.py
> +++ b/patchwork/parser.py
> @@ -922,7 +922,14 @@ def parse_mail(mail, list_id=None):
>          list_id (str): Mailing list ID
>  
>      Returns:
> -        None
> +        patch/cover letter/comment
> +        Or None if nothing is found in the mail
> +                   or X-P-H: ignore
> +                   or project not found
> +
> +    Raises:
> +        ValueError if there is an error in parsing
> +        Some db errors are passed through (e.g. IntegrityError)
>      """
>      # some basic sanity checks
>      if 'From' not in mail:
> -- 
> 2.14.1