Make user not nullable in DailyXFormSubmissionCounter #900
Conversation
| # Associate each daily counter with user=None with a user based on its xform | ||
| for counter in DailyXFormSubmissionCounter.objects.filter(user=None).iterator: | ||
| if counter.xform and counter.xform.user: | ||
| has_duplicate = DailyXFormSubmissionCounter.objects.filter( | ||
| date=counter.date, xform=counter.xform | ||
| ).exclude(user=None).exists() | ||
| # don't add a user to duplicate counters, so they get deleted in the next step | ||
| if not has_duplicate: | ||
| counter.user = counter.xform.user | ||
| counter.save() | ||
|
|
There was a problem hiding this comment.
I would prefer to use bulk_update() to be more efficient.
Something like that:
batch = []
batch_size = 5000
for counter in (
DailyXFormSubmissionCounter.objects.filter(user=None)
.exclude(xform=None)
.iterator(chunk_size=batch_size)
):
counter.user = counter.xform.user
if DailyXFormSubmissionCounter.objects.filter(
date=counter.date, xform=counter.xform
).exclude(user=None).exists():
continue
batch.append(counter)
if len(batch) >= batch_size:
DailyXFormSubmissionCounter.objects.bulk_update(batch, ['user_id'])
batch = []
if batch:
DailyXFormSubmissionCounter.objects.bulk_update(batch, ['user_id'])What do you think?
| # Delete daily counters without a user to avoid creating invalid monthly counters | ||
| DailyXFormSubmissionCounter.objects.filter(user=None).delete() | ||
|
|
There was a problem hiding this comment.
I'm wondering if those lines should be present in the 0031 too for users who have already run 0030 on an earlier date.
Example:
- Upgrade to
2.023.37g(which contains0030) on Oct 5. - Upgrade to
2.023.37h(which contains0031) on Oct 13.
0030 won't be reapplied and null could have been inserted between both patch releases.
What about adding adding a RunPython in 0031 which would block the migration if null is detected in user_id field? We could provided instruction to rollback to 0029 and upgrade to 0031. Otherwise, it would apply without any message.
There was a problem hiding this comment.
That's a very good point. I refactored 0030 to delete the daily counters and then repopulate the monthly counters, as well as creating a new 0031 migration that just deletes daily counters. The previous 0031 migration that alters the schema is now 0032.
This covers the edge case where a user upgraded to 2.023.37g on the same date as they upgrade to 2.023.37h. (Also, it means one less manual step needed to deploy.)
Makes the
userrelation onDailyXFormSubmissionCounternon-nullable, to make it matchMonthlyXFormSubmissionCounter.This fixes issues running
logger.0030_backfill_lost_monthly_counters. The migrations aren't squashed so that the schema alterations can be atomic.