Opened 9 months ago

Last modified 9 months ago

#22995 new Cleanup/optimization

Deprecate auto_now and auto_now_add and document alternatives

Reported by: jezdez 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 jezdez)

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 9 months ago by jezdez

  • Description modified (diff)

comment:2 Changed 9 months ago by aaugustin

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 9 months ago by kmtracey

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 9 months ago by aaugustin

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 9 months ago by timo

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.

comment:6 Changed 9 months ago by kmtracey

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 9 months ago by timo

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 9 months ago by kmtracey

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 9 months ago by kmtracey (previous) (diff)

comment:9 follow-ups: Changed 9 months ago by 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.

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 9 months ago by jezdez

+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 9 months ago by aaugustin

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 9 months ago by kmtracey

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 9 months ago by timo

  • Triage Stage changed from Unreviewed to Someday/Maybe
  • Version changed from 1.6 to master

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

comment:14 Changed 9 months ago by CollinAnderson

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 9 months ago by CollinAnderson (previous) (diff)

comment:15 Changed 9 months ago by CollinAnderson

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