Opened 2 years ago

Last modified 13 months ago

#22995 new Cleanup/optimization

Deprecate auto_now and auto_now_add and document alternatives

Reported by: Jannis Leidel Owned by: nobody
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords:
Cc: cmawebsite@… Triage Stage: Someday/Maybe
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Jannis Leidel)

We've previously documented some side issues of the two options (e.g. #6434) but need to better clarify the downsides of those magic parameters. The ability to override the save method as well as the default parameter has long made this implicit feature capable of being replaced.

This would probably also be a way to not let users fall in the trap as documented in #16745 and #22571.

In a previous *old* discussion there were multiple core developers in favor of the deprecation: https://groups.google.com/forum/#!topic/django-developers/TNYxwiXLTlI

Change History (15)

comment:1 Changed 2 years ago by Jannis Leidel

Description: modified (diff)

comment:2 Changed 2 years ago by Aymeric Augustin

AFAIK most core devs still agree that they should be removed. But it was never a priority because we simply avoid them in our projets ;-)

comment:3 Changed 2 years ago by Karen Tracey

I'm not really enthused about removing them, I agree with what Russell said here: https://code.djangoproject.com/ticket/12785#comment:1

In my experience they are (still) very commonly used. That original pre-1.0 discussion brings up that having to manually override save for every model where you have an auto-something field is going to be a pain in the neck, there was agreement on that, no specific alternative proposal got implemented and I don't see one here so we'd deprecate these w/ no replacement? That seems rude.

comment:4 Changed 2 years ago by Aymeric Augustin

I've seen several people go through the pain of replacing auto_now(_add) with the more explicit alternatives (default=timezone.now / override save()) after hitting bugs in production.

In fact, they weren't really bugs, but cases where the actual and documented behaviour diverged from what the developer expected — which has always been the rationale for removing them.

I would be fine with deprecating them (i.e. raise a DeprecationWarning) but never actually remove them (i.e. we wouldn't advance the deprecation at each release like we usually do). That would discourage their use without giving the finger to people who find them good enough.

comment:5 Changed 2 years ago by Tim Graham

I put together a proof of concept for removing usage of auto_add from the admin. I agree removing them would be disruptive. We have done a similar thing to what Aymeric suggested (deprecating, but not removing) with @permalink.

Last edited 13 months ago by Tim Graham (previous) (diff)

comment:6 Changed 2 years ago by Karen Tracey

Doesn't the POC replace an auto_now flag w/ a save override that is implementing auto_add_now? Which possibly does not matter since admin log entries are generally not editable...?

What are the unexpected behaviors that can be fixed by replacing with default/override save()? Or is it just more clear what the behavior will be?

comment:7 Changed 2 years ago by Tim Graham

Yes, I'd consider the fact that it currently uses auto_now and not auto_add_now to be a bug. It probably doesn't matter as you said, but if for some reason admin log entries are edited in a shell or whatever, I wouldn't expect the action_time to be updated.

The tickets linked in the description show some "gotchas" when these flags, you can also google "django auto_now evil" to find complaints. #22981 is also a related issue which stems from the fact that the expected behavior is not clearly defined (or at least it's poorly documented).

comment:8 Changed 2 years ago by Karen Tracey

Right, I do understand some of the gothcas, what I'm missing is if/how they are alleviated by users implementing their own? All of them I have seen it seems are still going to be a problem with self-implementation...? Hence my question on how this improves things, is is just (hopefully) clearer to the person doing it what the side-effects will be vs. a magical flag that does less than they think it will?

Last edited 2 years ago by Karen Tracey (previous) (diff)

comment:9 Changed 2 years ago by Russell Keith-Magee

If auto_now and auto_now_add have bugs, then we should address them; if there is potential for confusion around when they will (or won't) apply changes, we should document that too - just as we do when we note that update doesn't trigger pre/post save signals.

I simply don't understand the motivation of removing features that implement an obvious and well defined use case - creation timestamp and modification timestamp - without providing an equally functional alternative. In order to use default= as a replacement for auto_now_add, you need to:

  • Remember to use timezone.now, not datetime.now
  • Remember to use timezone.now, not timezone.now()
  • Remember to include django.utils import timezone

That's three easily avoidable opportunities for someone to mess up the simple use case of "creation timestamp". And it gets worse if you want auto_now, because you have to write and test a bunch of boilerplate code at the model level.

If there really is a visceral hatred of auto_now and auto_now_add as names, then at the very least we need to introduce a CreationDateTimeField, ModificationDateTimeField, CreationDateField, and ModificationDateField... but are those *really* that much better names than auto_now and auto_now_add?

comment:10 in reply to:  9 Changed 2 years ago by Jannis Leidel

+1 on marking it with DeprecationWarning but letting it sit there to ease the upgrading pain.

Replying to russellm:

If auto_now and auto_now_add have bugs, then we should address them; if there is potential for confusion around when they will (or won't) apply changes, we should document that too - just as we do when we note that update doesn't trigger pre/post save signals.

I simply don't understand the motivation of removing features that implement an obvious and well defined use case - creation timestamp and modification timestamp - without providing an equally functional alternative. In order to use default= as a replacement for auto_now_add, you need to:

  • Remember to use timezone.now, not datetime.now
  • Remember to use timezone.now, not timezone.now()
  • Remember to include django.utils import timezone

That's three easily avoidable opportunities for someone to mess up the simple use case of "creation timestamp". And it gets worse if you want auto_now, because you have to write and test a bunch of boilerplate code at the model level.

And yet, I believe knowing those steps are essential for any non-trivial Django project. Anecdotally I've often at some point converted the auto_now to a default call as soon as a model got a little more complex to use a consistent API for default values. I also consider overriding save methods not boilerplate but a core API of model classes -- again an often used one.

In that sense I believe the auto_now and auto_now_add parameters has not only actual technical issues, but also prevents our users to learn important APIs.

If there really is a visceral hatred of auto_now and auto_now_add as names, then at the very least we need to introduce a CreationDateTimeField, ModificationDateTimeField, CreationDateField, and ModificationDateField... but are those *really* that much better names than auto_now and auto_now_add?

Agreed, I think that would be a good compromise for users that don't want to manually handle auto_now and auto_now_add via default/save. There are CreationDateTimeField and ModificationDateTimeField fields in django-extensions in case we want to rely on a solution that's been in wide use for years.

comment:11 in reply to:  9 Changed 2 years ago by Aymeric Augustin

Replying to russellm:

I simply don't understand the motivation of removing features that implement an obvious and well defined use case - creation timestamp and modification timestamp - without providing an equally functional alternative.


Precisely, I think we should at least deprecate them because the use case is very obvious but auto_now(_add) don't implement it — and maybe can't!

Creation and modification timestamps are expected to be set automatically, regardless of how you access the row. Rails does this well at the database level.

comment:12 Changed 2 years ago by Karen Tracey

I'm not much of a fan of deprecating without removing either as I'm not a big fan of nanny warnings. If I know what I'm doing, have read the doc and understand the limitations of what I'm using, I don't really want the code nagging me that what I'm doing is "unclean". Bleh. Near as I can tell the straightforward do-it-yourself implementations are going to be equally vulnerable to gotchas...how is forcing people to try doing it themselves any better?

For the alternatives proposed: are they going to be immune from the gotcahs of the existing implementation? If not then why bother?

comment:13 Changed 2 years ago by Tim Graham

Triage Stage: UnreviewedSomeday/Maybe
Version: 1.6master

Moving to someday/maybe as this needs more investigation to answer the above questions before being accepted or rejected.

comment:14 Changed 2 years ago by Collin Anderson

We wouldn't ever consider adding a models.now() shortcut, would we? It's a little magical, but a very common use case. The import is what bugs me the most. You need to import something from "utils" in many of your models files. It seems similar to models.SET_NULL.

Or, have a magical models.DateField(default='now') (solves the now vs now()) or the longer: models.DateField(default=models.DateField.now)

Last edited 2 years ago by Collin Anderson (previous) (diff)

comment:15 Changed 2 years ago by Collin Anderson

Cc: cmawebsite@… added
Note: See TracTickets for help on using tickets.
Back to Top