Re: runserver fails on non-fatal model validation errors

[Shai Berger]

Hi all,

I've been going through what I've missed on the list, and found this 2-month-
old message that seems to have gone unanswered. I'm sorry for being so late to
the party, but...

On Saturday 23 March 2013, Brendan Jurd wrote:
>
> I reported an issue with runserver on the django trac [1], and aaugustin
> asked that I seek feedback here on the list.
>
> In short, the problem I am having is that exceptions thrown during model
> validation will cause runserver to abort. This might seem like a
> reasonable outcome, but as I discovered, there exist conditions that will
> raise an exception from model validation that are not actually fatal to the
> application.
>
> In my particular example, I am using a custom Manager on my object to
> produce an "extra" field, and then using that field in my "ordering" list.
> Model validation is complaining because this field isn't a declared Model
> field. This is a good thing for model validation to complain about, and I
> do want that message to be shown when I start up runserver.

Why? If you know it's not really a problem, why don't you want it silenced?
I understand that (as you mention on the ticket) you don't want to turn off
model validation -- I imagine there are plenty of other models to validate.
But why would you want to keep a false warning?

> But I don't
> want runserver to abort, because I happen to know that the model works
> exactly as intended with that configuration (and I don't know of any better
> alternative for making django ORDER BY an expression).
>

For reference, from the ticket: There is a field on the model that is a string,
and its content is a dot-delimited list of integers. The extra expression is
translating this field (in SQL) to an array of integers.

I see two possible paths to follow:

1) Define a custom field type, with the string representation (and, if you like,
python value) as the string of dot-separated integers, but with the db value
as the array. This may require other changes in your code, but I suspect it is
the more accurate database representation, so may also get you some benefits.

2) Since you're overriding the manager's get_query_set() anyway, Instead of
using the "ordering" meta option, just add the "order_by" clause to the
customized queryset. Admittedly, this is more of a workaround than a solution.

> I'm just a django user, not a developer, so I may be on the wrong track
> here, but I think runserver ought to catch validation exceptions, report
> them to the console, and then continue with its attempt to run the server.

I'm -1 on that. People tend not to look at the output of runserver unless you
force them to.

I'd be mildly supportive of a "pragma" -- say, an extra option in the Meta
class -- that tells validation to ignore some checks (or even the model in
general). Something like:

class YourModel(models.Model):
a = ...
...
class Meta:
ordering = ['b']
validation_ignore = ['ordering_fields_exist'] # Select validations
or
validation_ignore = True # Just skip this model

That is, I'd be supportive of it API-wise; I'm not sure the use-case justifies
the cost of the feature.

Another proper solution, IMO, is adding support for database-level calculated
fields (which would allow you to define your calculated field on the model,
avoiding the problems altogether). But this is not a simple thing, as each
database engine has its own expression language. Perhaps fields allowing
explicit SQL in their definition would work (in the sense that you'd be able to
use them on one database backend, at least). But that idea is completely
halfbaked.

> I included a patch for same in my original trac ticket report. In the
> worst case, it will end up running a server with models that don't work
> properly ... in which case the user has ignored the model validation errors
> at their own peril. Since runserver is not even intended for production
> use, I don't see much of a downside.
>
The downside is that, because people ignore the output of runserver, problems
which should have been discovered in development, will only by discovered in
much later (and more expensive) stages.

>
> [1] https://code.djangoproject.com/ticket/19126

HTH,
Shai.

[Brendan Jurd]

On Sunday, 5 May 2013 01:23:51 UTC+10, Shai Berger wrote:

I've been going through what I've missed on the list, and found this 2-month-
old message that seems to have gone unanswered. I'm sorry for being so late to
the party, but...

Thanks all the same for your reply -- better late than never.

Why? If you know it's not really a problem, why don't you want it silenced?
I understand that (as you mention on the ticket) you don't want to turn off
model validation -- I imagine there are plenty of other models to validate.
But why would you want to keep a false warning?

Well it's entirely possible that I might genuinely make that mistake (mentioning a non-existent field in 'ordering') somewhere in my app, and I'd like model validation to pick me up on that if it happens.  So I wouldn't want to shut that category of error up altogether.  If there were a reasonable way to make the model validation more discerning about this (look up the fields produced by the model's 'objects' manager, rather than just the fields defined on the model), that would be great, but I'm getting the impression that it wouldn't be worth pursuing for a weird corner case such as this.

1) Define a custom field type, with the string representation (and, if you like,
python value) as the string of dot-separated integers, but with the db value
as the array. This may require other changes in your code, but I suspect it is
the more accurate database representation, so may also get you some benefits.

A reasonable suggestion, but I do have other reasons for wanting the db value to be the text form.
 

2) Since you're overriding the manager's get_query_set() anyway, Instead of
using the "ordering" meta option, just add the "order_by" clause to the
customized queryset. Admittedly, this is more of a workaround than a solution.

I tried this, and can confirm that it gets me the result I'm looking for with no validation errors.  To be honest I don't know why I didn't think of it myself, but thank you for pointing it out.

I'd be mildly supportive of a "pragma" -- say, an extra option in the Meta
class --  that tells validation to ignore some checks (or even the model in
general). Something like:

        class YourModel(models.Model):
           a = ...
            ...
            class Meta:
               ordering = ['b']
               validation_ignore = ['ordering_fields_exist']  # Select validations
or
               validation_ignore = True  # Just skip this model

That is, I'd be supportive of it API-wise; I'm not sure the use-case justifies
the cost of the feature.

Yeah, does seem like overkill.  If there were widespread reports of problematic model validation errors, then it would make more sense, but mine is the only such report I've noticed.
 

Another proper solution, IMO, is adding support for database-level calculated
fields (which would allow you to define your calculated field on the model,
avoiding the problems altogether). But this is not a simple thing, as each
database engine has its own expression language. Perhaps fields allowing
explicit SQL in their definition would work (in the sense that you'd be able to
use them on one database backend, at least). But that idea is completely
halfbaked.

That sounds quite nice, actually.  If such a feature were available I'd definitely make use of it.

You're right that it wouldn't be portable across database backends, but then neither is my current solution using a customised queryset with 'extra'.

Cheers,
BJ