Hello Owais, I had flagged this for review but it didn’t make it to the top of my list until today, sorry.
The current approach adds 7 new APIs that only target the use case exposed in the ticket, which I find rather narrow. Some of these APIs (ignore_delegated) only exist to work around the inflexibility of others. This doesn’t sound like the best possible design. I think there’s a more generally useful set of APIs that could make your use case possible, as well as others. If I understood your PR correctly, you need five things, which I’m going to discuss below. 1) Ignore some fields on updates Django already provides three ways to do this: MyModel.objects.defer(‘ignored_1’, ‘ignored_2’).save() MyModel.objects.only(‘included_1’, ‘included_2’).save() MyModel.objects.save(update_fields=[‘included_1’, ‘included_2’]) (Of course, the second and third need something like included_fields = all_fields - excluded_fields.) Writing a custom manager to add the `defer()` clause automatically sounds preferable to adding a fourth way of doing the same thing, with a `delegated_on_update` keyword argument on fields. While we’re there, I don’t know if there’s a reason why `update_fields` can’t be replaced with `only()`. Both appear to achieve the same effet and `only()` is shorter. Could we deprecate `update_fields`? Technically, the replacement for `.save(update_fields=fields)` would be `.only(*fields).save(force_update=True)` because `update_fields` forces an update. But I suppose most people won’t bother with the `force_update` argument if `.only()` does the right thing both on inserts and updates. 2) Ignore some fields on inserts Since Django abstracts the insert/update distinction with its single API, `save()`, I’m reluctant to create insert_* / update_* APIs. I don’t know how `defer()` and `only()` behave on inserts. If they don’t have the same effect as on updates, I think we could consider it a bug and fix it. Combined with a deprecation of `update_fields` for the reasons explained above, this would leave just `.defer()` and `.only()`, a consistent and versatile API for controlling which fields are included or excluded by `.save()`. 3) Return some values on updates 4) Return some values on inserts I think a QuerySet-level method similar to `.only()` and `.defer()` would work here and would be more generally useful. Exposing this capability for databases that support it should be uncontroversial. I haven’t given it too much thought. Others probably have. 5) A reasonably DRY way to specify all this I think you should implement a custom default manager. This is the generic API Django gives to make this use case and many others possible. To sum up, I’m suggesting to remove one API instead of adding seven, to achieve pretty much the same result :-) I realize you already put a lot of work into the approach I’m discouraging. Sorry about that. I hadn’t noticed this ticket before. In any case, a long-winded discussion that results in a massive API is a clear sign that we mustn’t add one keyword argument for each possible behavior. Instead we must give tools for developers to describe the behavior they need in code. Best regards, -- Aymeric. > On 6 janv. 2016, at 16:04, Owais Lone <loneow...@gmail.com> wrote: > > Hi, I would really appreciate if someone could review the API design (and > implementation) for a PR I created. The PR implements support for fields that > are assigned a value by the database (using a default value or trigger) > instead of Django. It also adds support for PostgreSQL’s RETURNING and > Oracle’s RETURNING INTO clauses. > > This will allow us to have AutoField like behaviour on fields other than the > primary key or have primary key fields other than Integer and BigInteger > types. > > The PR also sets the stage for returning primary keys (and optionally other > fields) from the bulk_create operation on PostgreSQL and Oracle. > > Pull request: https://github.com/django/django/pull/5904 > Django ticket: https://code.djangoproject.com/ticket/21454 > > Thanks > > -- > 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/CE5C6CD8-146E-4680-89A7-1351DF0CA76F%40gmail.com. > For more options, visit https://groups.google.com/d/optout. -- 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/51E3E0EF-8D03-4906-AE21-58CBD9AA34FE%40polytechnique.org. For more options, visit https://groups.google.com/d/optout.
