Squash core migrations #528
Conversation
| model_name='image', | ||
| name='isic', | ||
| field=models.OneToOneField( | ||
| default=isic.core.models.isic_id.IsicId.safe_create, |
There was a problem hiding this comment.
This is actually a bug. When this AddField is executed by the migration engine, the default is evaluated, which has the side effect of inserting rows into the IsicId table.
This didn't manifest before because (apparently), an AlterField operation to add a default doesn't trigger the evaluation. 🤷
This made me realize that it's probably a bad idea to use IsicId.safe_create as the default. Any time an instance of Image is created locally, even if it's never saved, a new IsicId row is inserted. Instead, I think ideally we should explicitly create both the IsicId and the Image in the same transaction, so that if Image creation fails, the IsicId is removed. However, I'm not sure if its possible (or even makes logical sense) to do this in a way that is robust against race conditions in IsicId duplicates.
No description provided.