[PATCH 0/5] Remove 'Submission' model
Daniel Axtens
dja at axtens.net
Sun Mar 8 00:40:25 AEDT 2020
Stephen Finucane <stephen at 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 at 9bb03f73b714:~/patchwork$ python --version
Python 3.8.1
patchwork at 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
More information about the Patchwork
mailing list