Re: migration lock file implementation
To elaborate on this feature request: When working on a Django project with other people, a common issue is unintentionally creating conflicting migrations. These happen silently if two developers create and merge migrations in parallel. It's not clear there is a problem until after the second migration is merged. With this patch, the second migration will be prevented from merging by the VCS due to a merge conflict in the lock file. A documented example of a team that uses something like this in production can be found at the Zenefits engineering blog <https://engineering.zenefits.com/2015/11/using-old-ideas-to-solve-new-problems/> . Do you think that this is a useful feature to have in Django? And if so, does the proposed patch <https://github.com/django/django/compare/master...jianli:migration-lock-file> suffice? -Jian On Wednesday, May 17, 2017 at 1:30:41 PM UTC-7, Jian Li wrote: > > Hi! > > This is an idea that has been around for a while, and has been implemented > by various organizations running Django: using lock files to prevent > migration conflicts. > > I recently implemented something for our Django project at work. Here is a > cleaned-up version: > > > https://github.com/django/django/compare/master...jianli:migration-lock-file > > Hopefully this is useful enough to be merged upstream. Please let me know > what you think, and keep up the amazing work! > > -Jian > -- You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group. To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscr...@googlegroups.com. To post to this group, send email to django-developers@googlegroups.com. Visit this group at https://groups.google.com/group/django-developers. To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/98310105-15e4-473f-acab-e9e7b0427250%40googlegroups.com. For more options, visit https://groups.google.com/d/optout.
base implementations of natural key methods
Hi! In the course of implementing `natural_key` for many different models, I've noticed that the implementation is fairly predictable; it tends to use the fields already marked as unique. To avoid writing a separate implementation for each model, I've written a patch that implements the relevant logic on the model and manager base classes: https://github.com/jianli/django/compare/default-natural-key-implementation https://github.com/jianli/django/commit/b6d644b45c379cae83f7f2609525e616b62ade52 Details: - The proposed implementation is recursive, which enables it to create natural foreign keys even when the foreign key structure is arbitrarily deep. - When a natural key is not available but the user specifies `--natural-foreign`, the proposed implementation will raise an exception. This is inconsistent with the current behavior of silently falling back to using the "artificial" foreign key. However, I would argue that this is the correct behavior, and also the behavior implied by the documentation; if the user wants a natural foreign key serialization and Django is not able to provide it, Django should let the user know with an exception. - I was unsure whether to add the model method to `ModelBase` or `Model`. - I've manually tested a nearly identical patch against 1.5.4, but haven't had a chance to test against the development version. Please let me know what you think, and keep up the amazing work! -- You received this message because you are subscribed to the Google Groups "Django developers" group. To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscr...@googlegroups.com. To post to this group, send email to django-developers@googlegroups.com. Visit this group at http://groups.google.com/group/django-developers. To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CACwUGkzF7yrR30maQbA1602dLgWsqqS%2BH%2Bew-EhzjkJ%3D90-JfA%40mail.gmail.com. For more options, visit https://groups.google.com/d/optout.
Re: base implementations of natural key methods
Hi Russ,
Thanks for the feedback!
I believe the proposed code already addresses points 1-4.
1) The implementation does in fact return the correct natural key,
`(self.username,)`, for the User model.
2) When multiple fields have `unique=True`, the implementation does not
choose from among them; it returns a tuple containing *all*
non-auto-generated unique fields.
3) In the presence of multiple `unique_together` tuples, the implementation
arbitrarily chooses the first one.
4) In the presence of both `unique_together` fields and `unique` fields,
the implementation arbitrarily goes with the former.
You can see the specific algorithm at line
1422<https://github.com/jianli/django/commit/b6d644b45c379cae83f7f2609525e616b62ade52#diff-507b415116b409afa4f723e41a759a9eR1422>
.
The implementation always chooses a natural key which uniquely identifies
the relevant model. The choice may be arbitrary in the presence of multiple
cues, but so is any individually-implemented choice of a natural key.
At the very worst, if the user does not agree with the choice for the
specific model, they can fall back to individually implementing the natural
key for that model; this is no worse than the current situation. So I would
argue that the base implementation proposed here is strictly better than
the status quo.
In the course of serializing about a dozen different models, I've found
this to be a very useful base implementation that has saved me from the
tedium of writing many lines of boilerplate.
On Thu, May 8, 2014 at 10:07 AM, Russell Keith-Magee <
russ...@keith-magee.com> wrote:
>
> On Wed, May 7, 2014 at 10:20 AM, Jian Li wrote:
>
>> Hi!
>>
>> In the course of implementing `natural_key` for many different models,
>> I've noticed that the implementation is fairly predictable; it tends to use
>> the fields already marked as unique. To avoid writing a separate
>> implementation for each model, I've written a patch that implements the
>> relevant logic on the model and manager base classes:
>>
>>
>> https://github.com/jianli/django/compare/default-natural-key-implementation
>>
>> https://github.com/jianli/django/commit/b6d644b45c379cae83f7f2609525e616b62ade52
>>
>>
>> Details:
>>
>> - The proposed implementation is recursive, which enables it to create
>> natural foreign keys even when the foreign key structure is arbitrarily
>> deep.
>>
>> - When a natural key is not available but the user specifies
>> `--natural-foreign`, the proposed implementation will raise an exception.
>> This is inconsistent with the current behavior of silently falling back to
>> using the "artificial" foreign key. However, I would argue that this is the
>> correct behavior, and also the behavior implied by the documentation; if
>> the user wants a natural foreign key serialization and Django is not able
>> to provide it, Django should let the user know with an exception.
>>
>> - I was unsure whether to add the model method to `ModelBase` or `Model`.
>>
>> - I've manually tested a nearly identical patch against 1.5.4, but
>> haven't had a chance to test against the development version.
>>
>> Please let me know what you think, and keep up the amazing work!
>>
>
> Hi Jian,
>
> What you've proposed is an interesting idea, and a magnificent example of
> what you can do by exploiting the contents of _meta - but as a proposal for
> Django's core, I think it falls down in a couple of critical places.
>
> 1) A model without unique_together can still implement natural key. For
> example, the User model has a natural key, but doesn't have a
> unique_together clause. So, if you've just go a unique field, you can still
> have a natural key, but this helper won't help.
>
> 2) Assuming you can resolve (1) (which wouldn't be that hard in practice),
> multiple fields on a model can have unique=True set. How do you pick which
> one is *really* the natural key?
>
> 3) A model can have *multiple* unique_together clauses. Your code only
> appears to handle the case of "unique_together = ('field1','field2')", but
> it's also legitimate to say "unique_together = [('field1', 'field2'),
> ('field1', 'field3')]" - i.e., multiple independent unique_together
> clauses. Again, how do you pick which one is *really* the natural key?
>
> 4) What about if you have multiple fields with unique=True *and* multiple
> unique_together clauses?
>
> 5) Even if we could resolve those issues, there's a backwards
> compatibility issue implicit in changing behaviour around serialisation,
> and Django (as a project) is
migration lock file implementation
Hi! This is an idea that has been around for a while, and has been implemented by various organizations running Django: using lock files to prevent migration conflicts. I recently implemented something for our Django project at work. Here is a cleaned-up version: https://github.com/django/django/compare/master...jianli:migration-lock-file Hopefully this is useful enough to be merged upstream. Please let me know what you think, and keep up the amazing work! -Jian -- You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group. To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscr...@googlegroups.com. To post to this group, send email to django-developers@googlegroups.com. Visit this group at https://groups.google.com/group/django-developers. To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CACwUGkxYMkVtgoX6WgKDddV%2BihdpJUwe%2B1cos2E-RRsj3X3fjg%40mail.gmail.com. For more options, visit https://groups.google.com/d/optout.
