[PATCH 10/10] parsemail: Add print messages to help debugging email parsing

[Finucane, Stephen]

On 28 Nov 10:14, Mauro Carvalho Chehab wrote:
> When things are broken at parsemail, we need to be able to
> debug it. So, add some messages to it, in order to allow
> checking what it actually did, and to let it return 1 if
> an email got skipped by parsemail.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab at osg.samsung.com>

Some minor changes below - mostly dropping duplicated stuff and using
logging instead of 'print'. If you're happy for me to fix, I can do
so.

Stephen

> ---
>  patchwork/bin/parsemail.py | 19 +++++++++++++++----
>  patchwork/bin/parsemail.sh |  2 +-
>  2 files changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/patchwork/bin/parsemail.py b/patchwork/bin/parsemail.py
> index 5d6ddf45d264..71d284b273b0 100755
> --- a/patchwork/bin/parsemail.py
> +++ b/patchwork/bin/parsemail.py
> @@ -79,6 +79,9 @@ def find_project(mail):
>              except Project.DoesNotExist:
>                  pass
>  
> +    if project is None:
> +	project = Project.objects.get(listid = 'linux-media.vger.kernel.org')
> +

This is too specific to include - did you actually mean to include it?
BTW I'm open to ideas to provide a default project: the '--list-id'
parameter for this script (`./parsemail.py --help`) currently
overrides the search by header functionality - maybe this should change?

>      return project
>  
>  def find_author(mail):
> @@ -143,6 +146,9 @@ def find_pull_request(content):
>      match = git_re.search(content)
>      if match:
>          return match.group(1)
> +
> +    print "not a git pull request"
> +

This should be changed to LOG.debug (see below).

>      return None
>  
>  def try_decode(payload, charset):
> @@ -392,13 +398,16 @@ def parse_mail(mail):
>  
>      # some basic sanity checks
>      if 'From' not in mail:
> -        return 0
> +        print "From: is missing"
> +        return 1

I think this is a good idea. It's so good, in fact, that I did it
myself a few months back :) [1] There's some differences though
so the two efforts can be merged.

>  
>      if 'Subject' not in mail:
> -        return 0
> +        print "Subject: is missing"
> +        return 1
>  
>      if 'Message-Id' not in mail:
> -        return 0
> +        print "Message-Id: is missing"
> +        return 1
>  
>      hint = mail.get('X-Patchwork-Hint', '').lower()
>      if hint == 'ignore':
> @@ -407,7 +416,7 @@ def parse_mail(mail):
>      project = find_project(mail)
>      if project is None:
>          print "no project found"
> -        return 0
> +        return 1
>  
>      msgid = mail.get('Message-Id').strip()
>  
> @@ -431,6 +440,7 @@ def parse_mail(mail):
>          patch.delegate = delegate
>          try:
>              patch.save()
> +	    print "patch saved"
>          except Exception, ex:
>              print str(ex)
>  
> @@ -444,6 +454,7 @@ def parse_mail(mail):
>          comment.msgid = msgid
>          try:
>              comment.save()
> +	    print "comment saved"
>          except Exception, ex:
>              print str(ex)
>  
> diff --git a/patchwork/bin/parsemail.sh b/patchwork/bin/parsemail.sh
> index 9973392de9d4..c8220a799bba 100755
> --- a/patchwork/bin/parsemail.sh
> +++ b/patchwork/bin/parsemail.sh
> @@ -26,4 +26,4 @@ PYTHONPATH="$PATCHWORK_BASE":"$PATCHWORK_BASE/lib/python:$PYTHONPATH" \
>          DJANGO_SETTINGS_MODULE=patchwork.settings.production \
>          "$PATCHWORK_BASE/patchwork/bin/parsemail.py"
>  
> -exit 0
> +exit $@

Good idea - always wondered why we didn't make these exit codes mean
something.

> -- 
> 2.5.0

[1] https://github.com/getpatchwork/patchwork/commit/9a1802aa49299384e68d0fd2a3dd46c0885b29eb