| Message ID | 20180221141716.10908-8-dja@axtens.net |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Tools and fixes for parallel parsing | expand |
| Context | Check | Description |
|---|---|---|
| dja/snowpatch-0_1_0 | success | master/apply_patch Successfully applied |
| dja/snowpatch-snowpatch_job_snowpatch-patchwork | success | Test snowpatch/job/snowpatch-patchwork on branch master |
On 22/02/18 01:17, Daniel Axtens wrote: > Analysis of SQL statements showed that when parsing an email, the row > for the Person who sent the email was always getting updated. This is > because the test for updating it only checks if the incoming mail has > *a* name attached to the email address, and not if it has a new name. > Django is not smart enough to figure that out, and so unconditionally > UPDATEs the model when asked to save. > > Give it a hand - only update the model and save it if the new name is > in fact different. > > Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Though, just the != test should be sufficient here without the not-null test > --- > patchwork/parser.py | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/patchwork/parser.py b/patchwork/parser.py > index 1f3b3dd8901f..3d40b74375e0 100644 > --- a/patchwork/parser.py > +++ b/patchwork/parser.py > @@ -349,7 +349,7 @@ def get_or_create_author(mail): > # we lost the race to create the person > person = Person.objects.get(email__iexact=email) > > - if name: # use the latest provided name > + if name and name != person.name: # use the latest provided name > person.name = name > person.save() >
Andrew Donnellan <andrew.donnellan@au1.ibm.com> writes: > On 22/02/18 01:17, Daniel Axtens wrote: >> Analysis of SQL statements showed that when parsing an email, the row >> for the Person who sent the email was always getting updated. This is >> because the test for updating it only checks if the incoming mail has >> *a* name attached to the email address, and not if it has a new name. >> Django is not smart enough to figure that out, and so unconditionally >> UPDATEs the model when asked to save. >> >> Give it a hand - only update the model and save it if the new name is >> in fact different. >> >> Signed-off-by: Daniel Axtens <dja@axtens.net> > > Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> > > Though, just the != test should be sufficient here without the not-null test Wouldn't that mean a new null name would overwrite a previously saved name? Regards, Daniel > >> --- >> patchwork/parser.py | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/patchwork/parser.py b/patchwork/parser.py >> index 1f3b3dd8901f..3d40b74375e0 100644 >> --- a/patchwork/parser.py >> +++ b/patchwork/parser.py >> @@ -349,7 +349,7 @@ def get_or_create_author(mail): >> # we lost the race to create the person >> person = Person.objects.get(email__iexact=email) >> >> - if name: # use the latest provided name >> + if name and name != person.name: # use the latest provided name >> person.name = name >> person.save() >> > > -- > Andrew Donnellan OzLabs, ADL Canberra > andrew.donnellan@au1.ibm.com IBM Australia Limited
On 23/02/18 00:46, Daniel Axtens wrote: > Wouldn't that mean a new null name would overwrite a previously saved name? I... somehow didn't think about that case. Yes. Of course. Andrew > Regards, > Daniel >> >>> --- >>> patchwork/parser.py | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/patchwork/parser.py b/patchwork/parser.py >>> index 1f3b3dd8901f..3d40b74375e0 100644 >>> --- a/patchwork/parser.py >>> +++ b/patchwork/parser.py >>> @@ -349,7 +349,7 @@ def get_or_create_author(mail): >>> # we lost the race to create the person >>> person = Person.objects.get(email__iexact=email) >>> >>> - if name: # use the latest provided name >>> + if name and name != person.name: # use the latest provided name >>> person.name = name >>> person.save() >>> >> >> -- >> Andrew Donnellan OzLabs, ADL Canberra >> andrew.donnellan@au1.ibm.com IBM Australia Limited >
diff --git a/patchwork/parser.py b/patchwork/parser.py index 1f3b3dd8901f..3d40b74375e0 100644 --- a/patchwork/parser.py +++ b/patchwork/parser.py @@ -349,7 +349,7 @@ def get_or_create_author(mail): # we lost the race to create the person person = Person.objects.get(email__iexact=email) - if name: # use the latest provided name + if name and name != person.name: # use the latest provided name person.name = name person.save()
Analysis of SQL statements showed that when parsing an email, the row for the Person who sent the email was always getting updated. This is because the test for updating it only checks if the incoming mail has *a* name attached to the email address, and not if it has a new name. Django is not smart enough to figure that out, and so unconditionally UPDATEs the model when asked to save. Give it a hand - only update the model and save it if the new name is in fact different. Signed-off-by: Daniel Axtens <dja@axtens.net> --- patchwork/parser.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)