mbox series

[0/5] Remove 'Submission' model

Message ID 20200304115457.32300-1-stephen@that.guru
Headers show
Series Remove 'Submission' model | expand

Message

Stephen Finucane March 4, 2020, 11:54 a.m. UTC
Time to clean up the Submission-Patch-CoverLetter mess and unblock
features like Veronika's reworked tagging infrastructure series.

As discussed in the commit message, this series ulimately takes us from
this model hierarchy:

  Submission
    Patch
    CoverLetter
    Comment

To this hierarchy:

  Patch
    PatchComment
  Cover
    CoverComment

It does this by splitting out the cover letter-related entries from the
Submission-CoverLetter and Comment models into two entirely new models,
Cover and CoverComment, and then renaming Comment to PatchComment and
combining Patch and Submission. This is all explored in more detail in
the commit messages.

There were a couple of other approaches explored over the two years or
so of on-and-off investigation I've had into this issue, which I've
listed below:

- Add entirely new models

  Add new models that will replace Patch, CoverLetter and Comment, as
  well as Submission. These could be called Patch, Cover, PatchComment
  and CoverComment, (with some intermediate names to allow the old and
  new models to coexist while we migrate stuff across), so:

      Submission+Patch -> Patch
      Submission+CoverLetter -> Cover
      Comment -> PatchComment or CoverComment, depending on parent

  Rejected because of the sheer amount of data that has to be moved
  around for this to work. Effectively, the entire database would need
  to be rewritten.

- Drop CoverLetter in favour of Series

  A cover letter is essentially additional metadata for a series and
  could be folded into this.

  Rejected because of the impact on the API and UI since we'd no longer
  have cover letter IDs to use in the URL. Doesn't give us enough to
  justify the effort.

- Use generic relations for 'Comment'

  The contenttypes framework provided by Django allows you to map a
  model to another model. This means we could have a single Comment
  model that could point to either the Cover or Patch model.

  Rejected because I didn't think the complexity was worth it. I may
  revisit this though before we merge since we'll presumably want
  similar functionality for the reworked tags feature in the future.

- Merge Submission into Patch, rather than the other way around

  This was actually my first approach and initially seemed easier due to
  the amount of references to the Patch model.

  Rejected because this is not currently possible using stock Django due
  to bug #23521, which states that there is no way to change a model's
  'bases' attribute at the moment. I had to copy code from an unmerged
  PR, #11222, to get this to work which didn't seem ideal. In addition,
  it turned out updating the references wasn't difficult once I realized
  that another bug, #25530, only affected Django 1.11 and could be
  worked around.

Daniel: I'm thinking this could form the bulk of a 3.0 release,
alongside removal of Python 2.7 support and legacy Django version. This
appears to resolve Konstantin's DB murdering query, and should also make
better performance a more achievable goal for the patch relations work.
As such, I'd personally also like to hold off the latter until we get
this in.

[1] https://code.djangoproject.com/ticket/23521
[2] https://github.com/django/django/pull/11222
[3] https://code.djangoproject.com/ticket/25530

Stephen Finucane (5):
  trivial: Rename 'CoverLetter' references to 'Cover'
  Remove unnecessary references to Submission model
  Revert "Be sensible computing project patch counts"
  models: Split 'CoverLetter' from 'Submission'
  models: Merge 'Patch' and 'Submission'

 docs/api/schemas/latest/patchwork.yaml        |  14 +-
 docs/api/schemas/patchwork.j2                 |  14 +-
 docs/api/schemas/v1.0/patchwork.yaml          |  14 +-
 docs/api/schemas/v1.1/patchwork.yaml          |  14 +-
 docs/api/schemas/v1.2/patchwork.yaml          |  14 +-
 patchwork/admin.py                            |  28 +-
 patchwork/api/comment.py                      |  56 +++-
 patchwork/api/cover.py                        |  34 +--
 patchwork/api/embedded.py                     |   4 +-
 patchwork/api/event.py                        |   4 +-
 patchwork/api/filters.py                      |   8 +-
 patchwork/api/series.py                       |   4 +-
 patchwork/management/commands/dumparchive.py  |   2 +-
 patchwork/management/commands/parsearchive.py |  11 +-
 patchwork/migrations/0040_add_cover_model.py  | 249 ++++++++++++++++
 .../0041_merge_patch_submission_a.py          | 281 ++++++++++++++++++
 .../0042_merge_patch_submission_b.py          |  17 ++
 patchwork/models.py                           | 170 +++++++----
 patchwork/parser.py                           |  89 ++++--
 patchwork/signals.py                          |   4 +-
 patchwork/tests/api/test_comment.py           |  11 +-
 patchwork/tests/api/test_cover.py             |   2 +-
 patchwork/tests/test_detail.py                |  33 +-
 patchwork/tests/test_mboxviews.py             |  40 ++-
 patchwork/tests/test_parser.py                |   4 +-
 patchwork/tests/test_series.py                |   2 +-
 patchwork/tests/test_tags.py                  |   6 +-
 patchwork/tests/utils.py                      |  37 ++-
 patchwork/urls.py                             |   8 +-
 patchwork/views/__init__.py                   |   2 +-
 patchwork/views/comment.py                    |  43 ++-
 patchwork/views/cover.py                      |  22 +-
 patchwork/views/patch.py                      |  13 +-
 patchwork/views/project.py                    |  29 +-
 patchwork/views/utils.py                      |  21 +-
 35 files changed, 1020 insertions(+), 284 deletions(-)
 create mode 100644 patchwork/migrations/0040_add_cover_model.py
 create mode 100644 patchwork/migrations/0041_merge_patch_submission_a.py
 create mode 100644 patchwork/migrations/0042_merge_patch_submission_b.py

Comments

Daniel Axtens March 5, 2020, 12:37 a.m. UTC | #1
Stephen Finucane <stephen@that.guru> writes:

> Time to clean up the Submission-Patch-CoverLetter mess and unblock
> features like Veronika's reworked tagging infrastructure series.
>
> As discussed in the commit message, this series ulimately takes us from
> this model hierarchy:
>
>   Submission
>     Patch
>     CoverLetter
>     Comment
>
> To this hierarchy:
>
>   Patch
>     PatchComment
>   Cover
>     CoverComment
>
> It does this by splitting out the cover letter-related entries from the
> Submission-CoverLetter and Comment models into two entirely new models,
> Cover and CoverComment, and then renaming Comment to PatchComment and
> combining Patch and Submission. This is all explored in more detail in
> the commit messages.
>
> There were a couple of other approaches explored over the two years or
> so of on-and-off investigation I've had into this issue, which I've
> listed below:
>
> - Add entirely new models
>
>   Add new models that will replace Patch, CoverLetter and Comment, as
>   well as Submission. These could be called Patch, Cover, PatchComment
>   and CoverComment, (with some intermediate names to allow the old and
>   new models to coexist while we migrate stuff across), so:
>
>       Submission+Patch -> Patch
>       Submission+CoverLetter -> Cover
>       Comment -> PatchComment or CoverComment, depending on parent
>
>   Rejected because of the sheer amount of data that has to be moved
>   around for this to work. Effectively, the entire database would need
>   to be rewritten.
>
> - Drop CoverLetter in favour of Series
>
>   A cover letter is essentially additional metadata for a series and
>   could be folded into this.
>
>   Rejected because of the impact on the API and UI since we'd no longer
>   have cover letter IDs to use in the URL. Doesn't give us enough to
>   justify the effort.
>
> - Use generic relations for 'Comment'
>
>   The contenttypes framework provided by Django allows you to map a
>   model to another model. This means we could have a single Comment
>   model that could point to either the Cover or Patch model.
>
>   Rejected because I didn't think the complexity was worth it. I may
>   revisit this though before we merge since we'll presumably want
>   similar functionality for the reworked tags feature in the future.
>
> - Merge Submission into Patch, rather than the other way around
>
>   This was actually my first approach and initially seemed easier due to
>   the amount of references to the Patch model.
>
>   Rejected because this is not currently possible using stock Django due
>   to bug #23521, which states that there is no way to change a model's
>   'bases' attribute at the moment. I had to copy code from an unmerged
>   PR, #11222, to get this to work which didn't seem ideal. In addition,
>   it turned out updating the references wasn't difficult once I realized
>   that another bug, #25530, only affected Django 1.11 and could be
>   worked around.
>
> Daniel: I'm thinking this could form the bulk of a 3.0 release,
> alongside removal of Python 2.7 support and legacy Django version. This
> appears to resolve Konstantin's DB murdering query, and should also make
> better performance a more achievable goal for the patch relations work.
> As such, I'd personally also like to hold off the latter until we get
> this in.

It might, it also might not - part of the problem is jumping through
models and I'm not sure this is going to avoid that. I'll have a look.

I am leaning towards putting relations in 2.2 and seeing what happens:
you only get pathological performance if all of a sudden we start to see
huge numbers of relations and I don't think that's going to happen very
quickly given the state of tooling around relations.

Having said that, let me have a look at your series and see if I can get
both it and relations going, if it turns out to trivially solve our
performance problems I'm happy to wait.

Regards,
Daniel

>
> [1] https://code.djangoproject.com/ticket/23521
> [2] https://github.com/django/django/pull/11222
> [3] https://code.djangoproject.com/ticket/25530
>
> Stephen Finucane (5):
>   trivial: Rename 'CoverLetter' references to 'Cover'
>   Remove unnecessary references to Submission model
>   Revert "Be sensible computing project patch counts"
>   models: Split 'CoverLetter' from 'Submission'
>   models: Merge 'Patch' and 'Submission'
>
>  docs/api/schemas/latest/patchwork.yaml        |  14 +-
>  docs/api/schemas/patchwork.j2                 |  14 +-
>  docs/api/schemas/v1.0/patchwork.yaml          |  14 +-
>  docs/api/schemas/v1.1/patchwork.yaml          |  14 +-
>  docs/api/schemas/v1.2/patchwork.yaml          |  14 +-
>  patchwork/admin.py                            |  28 +-
>  patchwork/api/comment.py                      |  56 +++-
>  patchwork/api/cover.py                        |  34 +--
>  patchwork/api/embedded.py                     |   4 +-
>  patchwork/api/event.py                        |   4 +-
>  patchwork/api/filters.py                      |   8 +-
>  patchwork/api/series.py                       |   4 +-
>  patchwork/management/commands/dumparchive.py  |   2 +-
>  patchwork/management/commands/parsearchive.py |  11 +-
>  patchwork/migrations/0040_add_cover_model.py  | 249 ++++++++++++++++
>  .../0041_merge_patch_submission_a.py          | 281 ++++++++++++++++++
>  .../0042_merge_patch_submission_b.py          |  17 ++
>  patchwork/models.py                           | 170 +++++++----
>  patchwork/parser.py                           |  89 ++++--
>  patchwork/signals.py                          |   4 +-
>  patchwork/tests/api/test_comment.py           |  11 +-
>  patchwork/tests/api/test_cover.py             |   2 +-
>  patchwork/tests/test_detail.py                |  33 +-
>  patchwork/tests/test_mboxviews.py             |  40 ++-
>  patchwork/tests/test_parser.py                |   4 +-
>  patchwork/tests/test_series.py                |   2 +-
>  patchwork/tests/test_tags.py                  |   6 +-
>  patchwork/tests/utils.py                      |  37 ++-
>  patchwork/urls.py                             |   8 +-
>  patchwork/views/__init__.py                   |   2 +-
>  patchwork/views/comment.py                    |  43 ++-
>  patchwork/views/cover.py                      |  22 +-
>  patchwork/views/patch.py                      |  13 +-
>  patchwork/views/project.py                    |  29 +-
>  patchwork/views/utils.py                      |  21 +-
>  35 files changed, 1020 insertions(+), 284 deletions(-)
>  create mode 100644 patchwork/migrations/0040_add_cover_model.py
>  create mode 100644 patchwork/migrations/0041_merge_patch_submission_a.py
>  create mode 100644 patchwork/migrations/0042_merge_patch_submission_b.py
>
> -- 
> 2.24.1
Daniel Axtens March 7, 2020, 1:40 p.m. UTC | #2
Stephen Finucane <stephen@that.guru> writes:

> Time to clean up the Submission-Patch-CoverLetter mess and unblock
> features like Veronika's reworked tagging infrastructure series.
>
> As discussed in the commit message, this series ulimately takes us from
> this model hierarchy:
>
>   Submission
>     Patch
>     CoverLetter
>     Comment
>
> To this hierarchy:
>
>   Patch
>     PatchComment
>   Cover
>     CoverComment
>
> It does this by splitting out the cover letter-related entries from the
> Submission-CoverLetter and Comment models into two entirely new models,
> Cover and CoverComment, and then renaming Comment to PatchComment and
> combining Patch and Submission. This is all explored in more detail in
> the commit messages.

I'm having some errors doing the migration:

patchwork@9bb03f73b714:~/patchwork$ python --version
Python 3.8.1

patchwork@9bb03f73b714:~/patchwork$ python manage.py migrate
Traceback (most recent call last):
...
  File "<frozen importlib._bootstrap>", line 991, in _find_and_load
  File "<frozen importlib._bootstrap>", line 975, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 671, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 783, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "/home/patchwork/patchwork/patchwork/migrations/0041_merge_patch_submission_a.py", line 49, in <module>
    class Migration(migrations.Migration):
  File "/home/patchwork/patchwork/patchwork/migrations/0041_merge_patch_submission_a.py", line 251, in Migration
    index=models.Index(
  File "/opt/pyenv/versions/3.8.1/lib/python3.8/site-packages/django/db/models/indexes.py", line 31, in __init__
    self.fields_orders = [
  File "/opt/pyenv/versions/3.8.1/lib/python3.8/site-packages/django/db/models/indexes.py", line 32, in <listcomp>
    (field_name[1:], 'DESC') if field_name.startswith('-') else (field_name, '')
TypeError: startswith first arg must be bytes or a tuple of bytes, not str

Works if I do the migration with python2.
This also breaks 'docker-compose --test'

A set of small tweaks to the migration will fix it:

diff --git a/patchwork/migrations/0040_add_cover_model.py b/patchwork/migrations/0040_add_cover_model.py
index aa0ff5efb2ee..78283be0caf0 100644
--- a/patchwork/migrations/0040_add_cover_model.py
+++ b/patchwork/migrations/0040_add_cover_model.py
@@ -57,8 +57,8 @@ class Migration(migrations.Migration):
         migrations.AddIndex(
             model_name='cover',
             index=models.Index(
-                fields=[b'date', b'project', b'submitter', b'name'],
-                name=b'cover_covering_idx',
+                fields=['date', 'project', 'submitter', 'name'],
+                name='cover_covering_idx',
             ),
         ),
         migrations.AlterUniqueTogether(
@@ -108,7 +108,7 @@ class Migration(migrations.Migration):
         migrations.AddIndex(
             model_name='covercomment',
             index=models.Index(
-                fields=[b'cover', b'date'], name=b'cover_date_idx'
+                fields=['cover', 'date'], name='cover_date_idx'
             ),
         ),
         migrations.AlterUniqueTogether(
@@ -225,8 +225,8 @@ class Migration(migrations.Migration):
         migrations.AddIndex(
             model_name='comment',
             index=models.Index(
-                fields=[b'patch', b'date'],
-                name=b'patch_date_idx',
+                fields=['patch', 'date'],
+                name='patch_date_idx',
             ),
         ),
         migrations.AlterField(
diff --git a/patchwork/migrations/0041_merge_patch_submission_a.py b/patchwork/migrations/0041_merge_patch_submission_a.py
index 80b8dc2fe84d..002e276d01cb 100644
--- a/patchwork/migrations/0041_merge_patch_submission_a.py
+++ b/patchwork/migrations/0041_merge_patch_submission_a.py
@@ -250,15 +250,15 @@ class Migration(migrations.Migration):
             model_name='submission',
             index=models.Index(
                 fields=[
-                    b'archived',
-                    b'state',
-                    b'delegate',
-                    b'date',
-                    b'project',
-                    b'submitter',
-                    b'name',
+                    'archived',
+                    'state',
+                    'delegate',
+                    'date',
+                    'project',
+                    'submitter',
+                    'name',
                 ],
-                name=b'patch_covering_idx',
+                name='patch_covering_idx',
             ),
         ),
 

Regards,
Daniel

> There were a couple of other approaches explored over the two years or
> so of on-and-off investigation I've had into this issue, which I've
> listed below:
>
> - Add entirely new models
>
>   Add new models that will replace Patch, CoverLetter and Comment, as
>   well as Submission. These could be called Patch, Cover, PatchComment
>   and CoverComment, (with some intermediate names to allow the old and
>   new models to coexist while we migrate stuff across), so:
>
>       Submission+Patch -> Patch
>       Submission+CoverLetter -> Cover
>       Comment -> PatchComment or CoverComment, depending on parent
>
>   Rejected because of the sheer amount of data that has to be moved
>   around for this to work. Effectively, the entire database would need
>   to be rewritten.
>
> - Drop CoverLetter in favour of Series
>
>   A cover letter is essentially additional metadata for a series and
>   could be folded into this.
>
>   Rejected because of the impact on the API and UI since we'd no longer
>   have cover letter IDs to use in the URL. Doesn't give us enough to
>   justify the effort.
>
> - Use generic relations for 'Comment'
>
>   The contenttypes framework provided by Django allows you to map a
>   model to another model. This means we could have a single Comment
>   model that could point to either the Cover or Patch model.
>
>   Rejected because I didn't think the complexity was worth it. I may
>   revisit this though before we merge since we'll presumably want
>   similar functionality for the reworked tags feature in the future.
>
> - Merge Submission into Patch, rather than the other way around
>
>   This was actually my first approach and initially seemed easier due to
>   the amount of references to the Patch model.
>
>   Rejected because this is not currently possible using stock Django due
>   to bug #23521, which states that there is no way to change a model's
>   'bases' attribute at the moment. I had to copy code from an unmerged
>   PR, #11222, to get this to work which didn't seem ideal. In addition,
>   it turned out updating the references wasn't difficult once I realized
>   that another bug, #25530, only affected Django 1.11 and could be
>   worked around.
>
> Daniel: I'm thinking this could form the bulk of a 3.0 release,
> alongside removal of Python 2.7 support and legacy Django version. This
> appears to resolve Konstantin's DB murdering query, and should also make
> better performance a more achievable goal for the patch relations work.
> As such, I'd personally also like to hold off the latter until we get
> this in.
>
> [1] https://code.djangoproject.com/ticket/23521
> [2] https://github.com/django/django/pull/11222
> [3] https://code.djangoproject.com/ticket/25530
>
> Stephen Finucane (5):
>   trivial: Rename 'CoverLetter' references to 'Cover'
>   Remove unnecessary references to Submission model
>   Revert "Be sensible computing project patch counts"
>   models: Split 'CoverLetter' from 'Submission'
>   models: Merge 'Patch' and 'Submission'
>
>  docs/api/schemas/latest/patchwork.yaml        |  14 +-
>  docs/api/schemas/patchwork.j2                 |  14 +-
>  docs/api/schemas/v1.0/patchwork.yaml          |  14 +-
>  docs/api/schemas/v1.1/patchwork.yaml          |  14 +-
>  docs/api/schemas/v1.2/patchwork.yaml          |  14 +-
>  patchwork/admin.py                            |  28 +-
>  patchwork/api/comment.py                      |  56 +++-
>  patchwork/api/cover.py                        |  34 +--
>  patchwork/api/embedded.py                     |   4 +-
>  patchwork/api/event.py                        |   4 +-
>  patchwork/api/filters.py                      |   8 +-
>  patchwork/api/series.py                       |   4 +-
>  patchwork/management/commands/dumparchive.py  |   2 +-
>  patchwork/management/commands/parsearchive.py |  11 +-
>  patchwork/migrations/0040_add_cover_model.py  | 249 ++++++++++++++++
>  .../0041_merge_patch_submission_a.py          | 281 ++++++++++++++++++
>  .../0042_merge_patch_submission_b.py          |  17 ++
>  patchwork/models.py                           | 170 +++++++----
>  patchwork/parser.py                           |  89 ++++--
>  patchwork/signals.py                          |   4 +-
>  patchwork/tests/api/test_comment.py           |  11 +-
>  patchwork/tests/api/test_cover.py             |   2 +-
>  patchwork/tests/test_detail.py                |  33 +-
>  patchwork/tests/test_mboxviews.py             |  40 ++-
>  patchwork/tests/test_parser.py                |   4 +-
>  patchwork/tests/test_series.py                |   2 +-
>  patchwork/tests/test_tags.py                  |   6 +-
>  patchwork/tests/utils.py                      |  37 ++-
>  patchwork/urls.py                             |   8 +-
>  patchwork/views/__init__.py                   |   2 +-
>  patchwork/views/comment.py                    |  43 ++-
>  patchwork/views/cover.py                      |  22 +-
>  patchwork/views/patch.py                      |  13 +-
>  patchwork/views/project.py                    |  29 +-
>  patchwork/views/utils.py                      |  21 +-
>  35 files changed, 1020 insertions(+), 284 deletions(-)
>  create mode 100644 patchwork/migrations/0040_add_cover_model.py
>  create mode 100644 patchwork/migrations/0041_merge_patch_submission_a.py
>  create mode 100644 patchwork/migrations/0042_merge_patch_submission_b.py
>
> -- 
> 2.24.1
Daniel Axtens March 7, 2020, 2:14 p.m. UTC | #3
Daniel Axtens <dja@axtens.net> writes:

> Stephen Finucane <stephen@that.guru> writes:
>
>> Time to clean up the Submission-Patch-CoverLetter mess and unblock
>> features like Veronika's reworked tagging infrastructure series.
>>
>> As discussed in the commit message, this series ulimately takes us from
>> this model hierarchy:
>>
>>   Submission
>>     Patch
>>     CoverLetter
>>     Comment
>>
>> To this hierarchy:
>>
>>   Patch
>>     PatchComment
>>   Cover
>>     CoverComment
>>
>> It does this by splitting out the cover letter-related entries from the
>> Submission-CoverLetter and Comment models into two entirely new models,
>> Cover and CoverComment, and then renaming Comment to PatchComment and
>> combining Patch and Submission. This is all explored in more detail in
>> the commit messages.
>>
>> There were a couple of other approaches explored over the two years or
>> so of on-and-off investigation I've had into this issue, which I've
>> listed below:
>>
>> - Add entirely new models
>>
>>   Add new models that will replace Patch, CoverLetter and Comment, as
>>   well as Submission. These could be called Patch, Cover, PatchComment
>>   and CoverComment, (with some intermediate names to allow the old and
>>   new models to coexist while we migrate stuff across), so:
>>
>>       Submission+Patch -> Patch
>>       Submission+CoverLetter -> Cover
>>       Comment -> PatchComment or CoverComment, depending on parent
>>
>>   Rejected because of the sheer amount of data that has to be moved
>>   around for this to work. Effectively, the entire database would need
>>   to be rewritten.
>>
>> - Drop CoverLetter in favour of Series
>>
>>   A cover letter is essentially additional metadata for a series and
>>   could be folded into this.
>>
>>   Rejected because of the impact on the API and UI since we'd no longer
>>   have cover letter IDs to use in the URL. Doesn't give us enough to
>>   justify the effort.
>>
>> - Use generic relations for 'Comment'
>>
>>   The contenttypes framework provided by Django allows you to map a
>>   model to another model. This means we could have a single Comment
>>   model that could point to either the Cover or Patch model.
>>
>>   Rejected because I didn't think the complexity was worth it. I may
>>   revisit this though before we merge since we'll presumably want
>>   similar functionality for the reworked tags feature in the future.
>>
>> - Merge Submission into Patch, rather than the other way around
>>
>>   This was actually my first approach and initially seemed easier due to
>>   the amount of references to the Patch model.
>>
>>   Rejected because this is not currently possible using stock Django due
>>   to bug #23521, which states that there is no way to change a model's
>>   'bases' attribute at the moment. I had to copy code from an unmerged
>>   PR, #11222, to get this to work which didn't seem ideal. In addition,
>>   it turned out updating the references wasn't difficult once I realized
>>   that another bug, #25530, only affected Django 1.11 and could be
>>   worked around.
>>
>> Daniel: I'm thinking this could form the bulk of a 3.0 release,
>> alongside removal of Python 2.7 support and legacy Django version. This
>> appears to resolve Konstantin's DB murdering query, and should also make
>> better performance a more achievable goal for the patch relations work.
>> As such, I'd personally also like to hold off the latter until we get
>> this in.
>
> It might, it also might not - part of the problem is jumping through
> models and I'm not sure this is going to avoid that. I'll have a look.

The good news is that the relations work applies beautifully on top of
this series with only trivial conflict resolution and migration
renumbering. Good work, that's an impressively transparent refactoring.

The bad news is that it has absolutely no impact on the fetching of the
large fields. To avoid fetching the project every time, you have to do a
prefect_related(..., 'related__patches__project'). This pulls in the
entire patches model - including diff, content and headers - for all
related patches. This happens whether it's the old model or the new
model.

> I am leaning towards putting relations in 2.2 and seeing what happens:
> you only get pathological performance if all of a sudden we start to see
> huge numbers of relations and I don't think that's going to happen very
> quickly given the state of tooling around relations.

As a consequence, I'd still like to do this approach. If it goes
horribly wrong, it's straight-forward to patch out the whole API, make
it always return [] and reject all writes.

Regards,
Daniel

> Having said that, let me have a look at your series and see if I can get
> both it and relations going, if it turns out to trivially solve our
> performance problems I'm happy to wait.
>
> Regards,
> Daniel
>
>>
>> [1] https://code.djangoproject.com/ticket/23521
>> [2] https://github.com/django/django/pull/11222
>> [3] https://code.djangoproject.com/ticket/25530
>>
>> Stephen Finucane (5):
>>   trivial: Rename 'CoverLetter' references to 'Cover'
>>   Remove unnecessary references to Submission model
>>   Revert "Be sensible computing project patch counts"
>>   models: Split 'CoverLetter' from 'Submission'
>>   models: Merge 'Patch' and 'Submission'
>>
>>  docs/api/schemas/latest/patchwork.yaml        |  14 +-
>>  docs/api/schemas/patchwork.j2                 |  14 +-
>>  docs/api/schemas/v1.0/patchwork.yaml          |  14 +-
>>  docs/api/schemas/v1.1/patchwork.yaml          |  14 +-
>>  docs/api/schemas/v1.2/patchwork.yaml          |  14 +-
>>  patchwork/admin.py                            |  28 +-
>>  patchwork/api/comment.py                      |  56 +++-
>>  patchwork/api/cover.py                        |  34 +--
>>  patchwork/api/embedded.py                     |   4 +-
>>  patchwork/api/event.py                        |   4 +-
>>  patchwork/api/filters.py                      |   8 +-
>>  patchwork/api/series.py                       |   4 +-
>>  patchwork/management/commands/dumparchive.py  |   2 +-
>>  patchwork/management/commands/parsearchive.py |  11 +-
>>  patchwork/migrations/0040_add_cover_model.py  | 249 ++++++++++++++++
>>  .../0041_merge_patch_submission_a.py          | 281 ++++++++++++++++++
>>  .../0042_merge_patch_submission_b.py          |  17 ++
>>  patchwork/models.py                           | 170 +++++++----
>>  patchwork/parser.py                           |  89 ++++--
>>  patchwork/signals.py                          |   4 +-
>>  patchwork/tests/api/test_comment.py           |  11 +-
>>  patchwork/tests/api/test_cover.py             |   2 +-
>>  patchwork/tests/test_detail.py                |  33 +-
>>  patchwork/tests/test_mboxviews.py             |  40 ++-
>>  patchwork/tests/test_parser.py                |   4 +-
>>  patchwork/tests/test_series.py                |   2 +-
>>  patchwork/tests/test_tags.py                  |   6 +-
>>  patchwork/tests/utils.py                      |  37 ++-
>>  patchwork/urls.py                             |   8 +-
>>  patchwork/views/__init__.py                   |   2 +-
>>  patchwork/views/comment.py                    |  43 ++-
>>  patchwork/views/cover.py                      |  22 +-
>>  patchwork/views/patch.py                      |  13 +-
>>  patchwork/views/project.py                    |  29 +-
>>  patchwork/views/utils.py                      |  21 +-
>>  35 files changed, 1020 insertions(+), 284 deletions(-)
>>  create mode 100644 patchwork/migrations/0040_add_cover_model.py
>>  create mode 100644 patchwork/migrations/0041_merge_patch_submission_a.py
>>  create mode 100644 patchwork/migrations/0042_merge_patch_submission_b.py
>>
>> -- 
>> 2.24.1