[PATCH v2 2/3] models: Merge patch and first comment

[Andy Doan]

On 03/13/2016 03:50 PM, Stephen Finucane wrote:
> At the moment a patch is split into two model entries: a Patch and a
> linked Comment. The original rationale for this was that a Patch is
> really a sub-class of Comment. A comment is a record of the text
> content of an incoming mail, while a patch is that, plus the patch
> content too. Hence the separation and a one-to-one relationship when a
> patch is present. However, this decision was made before Django added
> support for model inheritance and is no longer necessary. This change
> flatten the models in preparation for some email subclassing work. This
> is achieved by copying over the non-duplicated fields from the Comment
> to the linked Patch, then deleting the Comment.
>
> The migrations are broken into two steps: a schema migration and a data
> migration, per the recommendations of the Django documentation [1]. SQL
> migration scripts, where necessary, will need to be created manually as
> there appears to be no way to do this in a way that is
> RDBMS-independant [2][3].
>
> [1] https://docs.djangoproject.com/en/1.9/topics/migrations/#data-migrations
> [2] https://stackoverflow.com/q/6856849/
> [3] https://stackoverflow.com/q/224732/
>
> Signed-off-by: Stephen Finucane <stephen.finucane at intel.com>

Reviewed-by: Andy Doan <andy.doan at linaro.org>

Not sure where or if its worth mentioning, but this migration is taking 
me around 30 minutes on my instance. So announcing a maintenance window 
is probably advisable.

> diff --git a/patchwork/migrations/0007_move_comment_content_to_patch_content.py b/patchwork/migrations/0007_move_comment_content_to_patch_content.py

> +def uncopy_comment_field(apps, schema_editor):
> +    Comment = apps.get_model('patchwork', 'Comment')

Comment is an unused variable.

> +def remove_duplicate_comments(apps, schema_editor):
> +    Comment = apps.get_model('patchwork', 'Comment')
> +    Patch = apps.get_model('patchwork', 'Patch')
> +
> +    for patch in Patch.objects.all():
> +        comment = Comment.objects.get(patch=patch, msgid=patch.msgid)
> +        comment.delete()

You need a try/except block here, eg:

         try:
             # when available, this can only return one entry due to the
             # unique_together constraint
             Comment.objects.get(patch=patch, msgid=patch.msgid).delete()
         except Comment.DoesNotExist:
             # though there's no requirement to actually have a comment
             continue

I see no need for a v3 once you've fixed this.