Stephen Finucane <step...@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 _______________________________________________ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork