[PATCH 0/5] Remove 'Submission' model
Stephen Finucane
stephen at that.guru
Wed Mar 4 22:54:52 AEDT 2020
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
--
2.24.1
More information about the Patchwork
mailing list