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

[Finucane, Stephen]

On 04 Jan 09:45, Finucane, Stephen wrote:
> 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?

Dropped this.

> >      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).

Changed to LOG.debug.

> >      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

Merged with above changes.