Getting errors with Series support

[Daniel Axtens]

Hi Daniel,

> I am using the current head of patchwork because of the fresh new Series 
> feature.

I am sorry to hear that things are broken.

> I don't want to send the complete error report to mailing list, though 
> private mail is fine for me. Is anyone interested in them?

The two best people would be Stephen F, as he wrote the Series series,
and myself as I did a lot of the reviews. I'd certainly be very
interested. Feel free to redact anything sensitive.

In terms of your errors:
> (1062, "Duplicate entry '143-5' for key 
> 'patchwork_seriespatch_series_id_1fc2c47f3eb85695_uniq'")

OK, so this is hitting our unique_together contstraint ('series',
'patch'): somehow we're trying to add another entry for the same series
and the same patch. I wonder why this has not triggered duplicate
detection at an earlier stage? Let me look into this more and get back
to you.

> (1048, "Column 'number' cannot be null")

So that looks like SeriesPatch's number field. SeriesPatches are only
created though the ManyToMany relationship between Patches and Series,
so we must be creating that in some dodgy way at some point.

That's mediated by Series.add_patch. That should really validate that a
number is provided - it could be given a None. (I guess we should
investigate static type hints!)

Series.add_patch is called in the parser (parse_mail), when a series can
be found, or is created. The number field takes the output of
parse_series_marker.

parse_series_marker should return either two numbers or (None, None). If
we're throwing an error, it must be returning (None, None).

Given that add_patch is guarded by 'if series:', we must have a
series. We have two options - when a series is found with find_series,
or when a series is created. Creation of a series is guarded by 'if not
series and n:', and we know n is None, so we must be hitting this error
not in the series creation path, but in the series found path.

So, we are hitting this error when there are no series markers found,
but find_series is finding a mail. This means we're passing None to
add_patch. I suspect we're seeing a patch in reply to a series, but
doesn't have a number. So something like:

        [PATCH 0/3] A cover letter
          [PATCH 1/3] The first patch
            A mail in reply that happens to contain a diff

We see mails with diffs get parsed as patches pretty often. I think we
were going to merge a patch which forced a mail beginning with Re: to be
parsed as a comment rather than a patch, but I'm not sure if that made
it in yet.

So I think we want to just say if the mail doesn't contain a number, we
should not add it to the series. I'll send a patch shortly.

Regards,
Daniel

>
> cheers,
> daniel
> _______________________________________________
> 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/20161116/86982835/attachment.sig>