[PATCH 8/9] parser: use Patch.objects.create instead of save()

[Andrew Donnellan]

On 22/02/18 01:17, Daniel Axtens wrote:
> Attempts to do parallel parsing with MySQL threw the following errors:
> 
> _mysql_exceptions.OperationalError: (1213, 'Deadlock found when trying to get lock; try restarting transaction')
> 
> Looking at the code, it was thrown when we created a patch like this:
> 
> patch = Patch(...)
> patch.save()
> 
> The SQL statements that were being generated were weird:
> 
> UPDATE "patchwork_patch" SET ...
> INSERT INTO "patchwork_patch" (...) VALUES (...)
> 
> As far as I can tell, the update could never work, because it was
> trying to update a patch that didn't exist yet. My hypothesis is
> that Django somehow didn't quite 'get' that because of the backend
> complexity of the Patch model, so it tried to do an update, failed,
> and then tried an insert.

Backend complexity... subclassing bug or something? Hmm.

> 
> Change the code to use Patch.objects.create, which makes the UPDATEs
> and the weird MySQL errors go away.
> 
> Also move it up a bit earlier in the process so that if things go wrong
> later at least we've committed the patch to the db.
> 
> Signed-off-by: Daniel Axtens <dja at axtens.net>

In any case, objects.create() is stylistically nicer imho.

I definitely think we should commit the patch to the database before 
Series...

Reviewed-by: Andrew Donnellan <andrew.donnellan at au1.ibm.com>

> ---
>   patchwork/parser.py | 29 ++++++++++++++---------------
>   1 file changed, 14 insertions(+), 15 deletions(-)
> 
> diff --git a/patchwork/parser.py b/patchwork/parser.py
> index 3d40b74375e0..0e53e6b9a3af 100644
> --- a/patchwork/parser.py
> +++ b/patchwork/parser.py
> @@ -984,6 +984,20 @@ def parse_mail(mail, list_id=None):
>               filenames = find_filenames(diff)
>               delegate = find_delegate_by_filename(project, filenames)
> 
> +        patch = Patch.objects.create(
> +            msgid=msgid,
> +            project=project,
> +            name=name[:255],
> +            date=date,
> +            headers=headers,
> +            submitter=author,
> +            content=message,
> +            diff=diff,
> +            pull_url=pull_url,
> +            delegate=delegate,
> +            state=find_state(mail))
> +        logger.debug('Patch saved')
> +
>           # if we don't have a series marker, we will never have an existing
>           # series to match against.
>           series = None
> @@ -1024,21 +1038,6 @@ def parse_mail(mail, list_id=None):
>                   except SeriesReference.DoesNotExist:
>                       SeriesReference.objects.create(series=series, msgid=ref)
> 
> -        patch = Patch(
> -            msgid=msgid,
> -            project=project,
> -            name=name[:255],
> -            date=date,
> -            headers=headers,
> -            submitter=author,
> -            content=message,
> -            diff=diff,
> -            pull_url=pull_url,
> -            delegate=delegate,
> -            state=find_state(mail))
> -        patch.save()
> -        logger.debug('Patch saved')
> -
>           # add to a series if we have found one, and we have a numbered
>           # patch. Don't add unnumbered patches (for example diffs sent
>           # in reply, or just messages with random refs/in-reply-tos)
> 

-- 
Andrew Donnellan              OzLabs, ADL Canberra
andrew.donnellan at au1.ibm.com  IBM Australia Limited