Opened 17 months ago

Closed 14 months ago

Last modified 14 months ago

#21905 closed New feature (fixed)

Add check for default=now() on Date/Time/DateTimeFields

Reported by: taishi@… Owned by: MarkusH
Component: Core (Other) Version: 1.7-alpha-1
Severity: Normal Keywords: check nlsprint14
Cc: info@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

I'm running a model which contains datetime.now() as default value and django keeps thinking that I altered that field.
So everytime I run migrates I get:
Running migrations:

No migrations needed.
Your models have changes that are not yet reflected in a migration, and so won't be applied.
Run 'manage.py makemigrations' to make new migrations, and then re-run 'manage.py migrate' to apply them.

If I run makemigrations (once, twice, 20 times) it will always output:
Migrations for 'acb_coins':

0003_auto_20140129_1947.py:

  • Alter field last_sync on coin

Model:
class Coin(models.Model):

name = models.CharField(max_length=80)
short_name = models.CharField(max_length=4)
rpc_address = models.CharField(max_length=30)
rpc_port = models.IntegerField(max_length=5)
balance = models.DecimalField(decimal_places=10, max_digits=15)
last_sync = models.DateTimeField(default=datetime.now())

Change History (18)

comment:1 Changed 17 months ago by matt@…

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

You don't want to be using datetime.now(): that gets evaluated at the time the module is imported.

You'll want to use datetime.now

comment:2 Changed 17 months ago by MarkusH

  • Cc info@… added

comment:3 Changed 17 months ago by taishi@…

matt@ comment worked

comment:4 Changed 17 months ago by MarkusH

  • Component changed from Migrations to Core (Other)
  • Keywords check added; migrate migrations datetime removed
  • Owner set to nobody
  • Summary changed from django migrate/makemigrations detects changes when none were made to Add check for default=now() on Date/Time/DateTimeFields

Given the confusion about passing default a datetime object instead of a callable (which solves the problem), I'd like to propose the idea to add a check to Django's check framework to make users aware of these problems that might occur over the time when using datetime.now() instead of datetime.now.

comment:5 Changed 17 months ago by mjtamlyn

  • Triage Stage changed from Unreviewed to Accepted

Accepted in principle, but I'm not sure how this could be implemented.

comment:6 Changed 17 months ago by matheusrosa

Why don't you use auto_now=True ? Every time you call save(), the last_sync field gets updated with the latest date and time.

Version 0, edited 17 months ago by matheusrosa (next)

comment:7 Changed 17 months ago by russellm

@mjtamlyn - This could be a job for the check framework. We could add a field-level check on the value of default - if the value of default is within some tolerance of now (e.g., within 10 seconds), raise a warning. Since the checks are run almost immediately after the model is loaded, any usage of datetime.now() would return the current date time (or as close as practical). The only way this check would fail would be if someone actually wanted *now*; so, we add it as a warning that can be silenced.

comment:8 Changed 17 months ago by russellm

  • Easy pickings set

To that end, it's a pretty easy fix; marking as such.

comment:9 Changed 17 months ago by MarkusH

Sounds like a plan, russellm. Do we want to get it into 1.7?

comment:10 Changed 17 months ago by russellm

@MarkusH - No reason it couldn't be in 1.7 - it's a minor fix. We just need a patch :-)

comment:11 Changed 17 months ago by rkstapenhurst

  • Resolution set to duplicate
  • Status changed from new to closed

Duplicate of #21638

comment:12 Changed 17 months ago by MarkusH

  • Resolution duplicate deleted
  • Status changed from closed to new

This ticket is not a duplicate at all. The intention is to add a check command warning a user that now() instead of now as a field's default value is probably not what they want.

comment:13 Changed 17 months ago by mamun

  • Owner changed from nobody to mamun
  • Status changed from new to assigned

comment:14 Changed 17 months ago by AeroNotix

@mamun patch incoming?

comment:15 Changed 17 months ago by MarkusH

  • Keywords nlsprint14 added
  • Owner changed from mamun to MarkusH

comment:16 Changed 17 months ago by MarkusH

  • Has patch set

comment:17 Changed 14 months ago by Markus Holtermann <info@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In 9d8c73f6a1c636853a5c5013f21985d702b2301b:

Fixed #21905 -- Add info message if DateField or TimeField use a fixed value

comment:18 Changed 14 months ago by Florian Apolloner <florian@…>

In 11932e978f980846d2c3326ff070ece7e65bf75c:

Merge pull request #2346 from Markush2010/ticket21905

Fixed #21905 -- Add info message if DateField or TimeField use a fixed value

Note: See TracTickets for help on using tickets.
Back to Top