Django 1.9: default='' no longer permitted on model field (with blank=False)

[James Addison]

I have a model like so:

class Provider(models.Model):
    description = models.TextField(default='')

In recent Django 1.9 changes (this did not occur with 1.8.x), this causes the following traceback:

Performing system checks...


Traceback (most recent call last):
  File "/home/vagrant/.virtualenvs/myproject/lib/python3.4/site-packages/gevent/greenlet.py", line 523, in run
    result = self._run(*self.args, **self.kwargs)
  File "/home/vagrant/.virtualenvs/myproject/src/django/django/utils/autoreload.py", line 226, in wrapper
    fn(*args, **kwargs)
  File "/home/vagrant/.virtualenvs/myproject/src/django/django/core/management/commands/runserver.py", line 116, in inner_run
    self.check(display_num_errors=True)
  File "/home/vagrant/.virtualenvs/myproject/src/django/django/core/management/base.py", line 472, in check
    raise SystemCheckError(msg)
django.core.management.base.SystemCheckError: SystemCheckError: System check identified some issues:


ERRORS:
businesses.Provider.description: (fields.E008) Invalid 'default' value: This field cannot be blank.
images.ImageSet.hash: (fields.E008) Invalid 'default' value: This field cannot be blank.
users.User.gender: (fields.E008) Invalid 'default' value: This field cannot be blank.


System check identified 3 issues (0 silenced).

Why is it that this was perfectly allowable before but not now? I have a data processing script that's pulling in data from an external source and creating instances with blank 'descriptions', it now doesn't work.  I have to set `blank=True` on fields having this error, which I don't want to do (for form validation reasons).

I believe the following links are pertinent:

https://code.djangoproject.com/ticket/25417

https://github.com/django/django/pull/5303

James

[Marc Tamlyn]

I disagree with this system check and I would like to see it reverted before 1.9 final.

I admit that my opinions here are skewed by the fact that I think model level validation is fundamentally not well implemented by Django at all. I work to a philosophy that there are two places where validation happens - once at the entry point to the system (form or serialiser) and once at the database level to ensure integrity of the data.

I view model level validators, custom clean methods, and the editable and blank flags as merely hints to automatic ModelForm creation and other similar objects. I can't think of a single time I have explicitly called model.clean(). Now, I can see the merits of a good model level validation system, Django's model level validation is trivial to bypass and I consider relying on it to be bad practice.

This check implies that you must use model level validation. I don't agree it should be that unavoidable a feature. The docs for `blank` in fact only reference form validation https://docs.djangoproject.com/en/1.8/ref/models/fields/#blank.

The system check framework is supposed to be there to stop easy mistakes. However:

- The original error reported by the OP of the ticket was in fact not able to accidentally deploy the code in question because it did not run.

- It is common practice to not be use or at least be perfect about your model level validation. I certainly have created many fields which are system level but do not have `editable=False` on them because I explicitly make sure all of my forms do not include this field. They are likely hidden away in the admin as well because they are only used by some internal code. Best practices such as the requirement for model forms to have fields specified make sure that I am being explicit.

- If we are going to make it required that your model level validation is correct, then we need to do so in a much more complete way than just checking default values. In particular, code such as Manager.create() should check validation rules. This would be a hugely breaking change for Django.

I can of course easily bypass this change, add it to ignored checks or similar. However as a beginner developer I would see this check, think model level validation is important and reliable.

Marc

--
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-develop...@googlegroups.com.
To post to this group, send email to django-d...@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/c6ffdd4b-6147-43ff-be73-daef2e8149fe%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Russell Keith-Magee]

On Sat, Oct 24, 2015 at 5:42 AM, Marc Tamlyn <marc....@gmail.com> wrote:

I disagree with this system check and I would like to see it reverted before 1.9 final.

I agree - I’d argue that this check is demonstrably *incorrect*. 

 

I admit that my opinions here are skewed by the fact that I think model level validation is fundamentally not well implemented by Django at all. I work to a philosophy that there are two places where validation happens - once at the entry point to the system (form or serialiser) and once at the database level to ensure integrity of the data.

I don’t think you even need to bring model validation into this discussion - the two keywords in play here (blank and default) are for entirely different parts of Django. blank is display/form logic. default is model logic. There’s some crossover to be sure, but the use case presented by James is entirely legal and valid for exactly that reason.

So, +1 - this should be reverted. As far as I can make out, it’s not even correct to say that this is a warning - the model description makes perfect sense as-is. The only way I can see that this check would be valid is a confirmation of blank=False and a model-level non-zero length validator - but that’s getting sufficiently eclectic that I don’t think it warrants inclusion.

Yours,

Russ Magee %-)

[Tim Graham]

Maybe it would be enough to ignore ValidationErrors with code='blank' and/or code='null' in the check? That's been the only "false positive" with this check as far as I can tell.

[Tim Graham]

In addition, downgrading it from "error" to "warning" would be okay with me, too).  If we had some sort of "# NOQA" mechanism like flake8 has to more granularly ignore system check warnings, that could be nifty too.

I'll admit I might be overly enthusiastic about using the checks framework to try to prevent invalid tickets from newbies making mistakes. It's difficult to anticipate the universe of valid use cases, so thanks for testing the prerelease and offering your feedback.

[charettes]

Hi everyone I'm the author of this change.

I'm not a big fan of the model validation layer myself but I assumed basing a model level check on it would make sense. I was not aware there was such a bad feeling about the model level validation, if I had I wouldn't have based this check on it.

It was a controversial change from the beginning and the process of adding this check should have taken place in it's own ticket.

I also think this check could be adjusted to ignore code='blank' and 'null' and turned into a warning, I don't believe it is entirely flawed as report have shown (https://code.djangoproject.com/ticket/25480#comment:3). It wouldn't be the first time a newly added check requires adjustments.

I submitted a PR (https://github.com/django/django/pull/5471) to completely revert this change given suggest adjustments don't get enough support.

Thanks for your feedback on this prerelease,

Simon

[Shai Berger]

Hi,

On Saturday 24 October 2015 04:13:47 charettes wrote:
> Hi everyone I'm the author of this change.
>

> I submitted a PR (https://github.com/django/django/pull/5471) to completely
> revert this change given suggest adjustments don't get enough support.
>

I don't think that's the right fix. I think a field whose default value fails
its "clean" method is bogus and should be flagged. A warning for sure, an error
is not insensible.

However, I'm very surprised to find out that a model field's "clean()" method
even looks at the "blank" attribute. Like others on this thread, I was under
the impression that it's only for model-form generation.

Shai.

[charettes]

Hi everyone,

I proceeded to revert the commit for now.

Nothing prevent us from adding a warning in the future if we ever fix the whole "blank" issue at the model level.

Simon