Opened 11 years ago

Closed 10 years ago

Last modified 3 years ago

#19126 closed New feature (fixed)

Allow runserver to bypass model validation

Reported by: Brendan Jurd Owned by: nobody
Component: Core (Management commands) Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I recently encountered a frustrating little issue in one of my apps. I wasn't able to run the development server because of a non-critical model validation failure.

The error was '"ordering" refers to "split_code", a field that doesn't exist.'

In several of my models, "ordering" refers to a field which isn't part of the main model definition, but *is* added to the query set by a custom default Manager. So in practice (when I run it under mod_wsgi) the ordering works exactly as intended. This is a case where the model fails to validate, but actually works properly in a live-fire situation.

Perhaps the model validation is too strict in this case, but I can understand why. It would probably be a lot of fuss to try to make it clever enough to detect additional fields that are pulled in by the queryset on the Manager. But there's a broader issue in that *any* model validation failure, no matter how unimportant, is enough to stop the development server from working with an "Unhandled exception" in 1.3.1.

I think it's fine that I get a message about the "ordering" definition, but I don't think it should be a showstopper.

There are few different ways to get past this. My suggestion would be that when executing "runserver", we should catch CommandErrors coming out of 'validate', report them, and then keep on trying to run the server anyway.

I've included a patch along these lines.

Other options include

  • teach 'validate' to recognise fields that are included by the default Manager's queryset,
  • teach 'validate' to differentiate between errors that are critical in nature, and those that really only need to be warnings,
  • allow SQL expressions in 'ordering', not just field names, so that the custom Manager field isn't necessary in the first place.

... but they all seem like a lot of effort.

Thanks for your time.

Attachments (1)

runserver-validate.diff (898 bytes ) - added by Brendan Jurd 11 years ago.
Patch to catch and report validation errors in runserver.

Download all attachments as: .zip

Change History (14)

by Brendan Jurd, 11 years ago

Attachment: runserver-validate.diff added

Patch to catch and report validation errors in runserver.

comment:1 by Łukasz Rekucki, 11 years ago

The problem is, there are parts of Django that do not use a default manager, but a base manager, which is always a Manager instance and will not return QuerySet with your custom field. I suspect (but didn't check) that YourModel._base_manager.filter() will fail then, but you just didn't hit that functionality.

in reply to:  1 comment:2 by Brendan Jurd, 11 years ago

Replying to lrekucki:

The problem is, there are parts of Django that do not use a default manager, but a base manager, which is always a Manager instance and will not return QuerySet with your custom field. I suspect (but didn't check) that YourModel._base_manager.filter() will fail then, but you just didn't hit that functionality.

I tried ._base_manager.filter(), and it didn't seem to have a problem with my 'ordering' trick. In fact, ._base_manager is an instance of my custom manager, not of Manager.

I may be using the terminology incorrectly, but when I said "default manager", I meant that I had defined my own Manager subclass with use_for_related_fields = True, and a custom get_query_set method, and then assigned an instance of it to the 'objects' property of the model class.

It might help if I explain my use-case. I have several tables which use a dotted integer format as their primary identifier, e.g., "3.1.1.12". In the database these are of text type, which is fine except for sorting, where I want them to sort numerically on each integer in turn, not lexicographically (3.1.1.9 should sort before 3.1.1.12).

If I were doing everything in SQL, this would be no big deal, I'd just write ORDER BY string_to_array(code, '.')::int[].

In Django, as far as I know you can't order by arbitrary SQL expressions, only fields. So I needed to get the code in its integer-array form in there as a "field" of sorts. So that's why I cooked up the custom Manager with .extra(select={'split_code': "string_to_array(account.code, '.')::int[]"}) added to get_query_set, and then set ordering = ['split_code'] in the model.

If there's a more Djangoistic way to approach this, I'm all ears. I could define a custom data type with comparison functions for sorting in the database, but that seems a bit like overkill.

Setting all that aside, though, I think my original point about model validation and runserver is still sane. What's the upside of pre-emptively killing runserver? If the model validation problems are severe enough to crash the app, then they'll crash the app and you'll get a shiny debug page. If they're not severe enough to crash the app then they are warnings.

comment:3 by Aymeric Augustin, 11 years ago

Triage Stage: UnreviewedDesign decision needed
Type: UncategorizedBug

One might try to work around this by inheriting runserver.Command and disabling validation, but that requires copying inner_run, a 50-lines method, just to remove the call to validate.

comment:4 by anonymous, 11 years ago

This bug effectively kills using Celery's django integration http://pypi.python.org/pypi/django-celery since it uses manage.py to start the Celery workers.

comment:5 by Karen Tracey, 11 years ago

It is djcelery itself that is requesting model validation on its command, e.g. for celeryd: https://github.com/celery/django-celery/blob/master/djcelery/management/commands/celeryd.py#L19

comment:6 by Aymeric Augustin, 11 years ago

Triage Stage: Design decision neededAccepted

runserver currently contains this code:

    # Validatioisn  called explicitly each time the server is reloaded.
    requires_model_validation = False

I propose to change this to True, and take it into account in inner_run.

Untested patch:

--- a/django/core/management/commands/runserver.py
+++ b/django/core/management/commands/runserver.py
@@ -30,9 +30,6 @@ class Command(BaseCommand):
     help = "Starts a lightweight Web server for development."
     args = '[optional port number, or ipaddr:port]'
 
-    # Validation is called explicitly each time the server is reloaded.
-    requires_model_validation = False
-
     def get_handler(self, *args, **options):
         """
         Returns the default WSGI handler for the runner.
@@ -89,7 +86,8 @@ class Command(BaseCommand):
         quit_command = (sys.platform == 'win32') and 'CTRL-BREAK' or 'CONTROL-C'
 
         self.stdout.write("Validating models...\n\n")
-        self.validate(display_num_errors=True)
+        if self.requires_model_validation:
+            self.validate(display_num_errors=True)
         self.stdout.write((
             "%(started_at)s\n"
             "Django version %(version)s, using settings %(settings)r\n"

That would make it trivial to disable model validation when needed.

in reply to:  6 comment:7 by direvus@…, 11 years ago

Replying to aaugustin:

That would make it trivial to disable model validation when needed.

That seems like a good idea, but it wouldn't really solve the problem I originally posted about. I can already disable model validation, but I'd rather not. What I want is for model validation to run, and report its findings, but not to prevent runserver from starting up.

Model validation is great! I want it to do its thing. The issue is that runserver's response to any model validation problem at all, is to commit immediate suicide. If all model validation problems were of a fatal nature, that would make sense. But they're not, so it doesn't.

comment:8 by Aymeric Augustin, 11 years ago

Can you bring this up on django-developers? If other users are affected by this issue, their feedback will be interesting.

in reply to:  8 comment:9 by Brendan Jurd, 11 years ago

Replying to aaugustin:

Can you bring this up on django-developers? If other users are affected by this issue, their feedback will be interesting.

No problem -- I have posted my message but it hasn't shown up on the list yet, I suppose it's waiting for moderation.

comment:10 by Tim Graham, 10 years ago

Summary: runserver versus model validationAllow runserver to bypass model validation
Type: BugNew feature
Version: 1.3master

comment:11 by Tim Graham, 10 years ago

Resolution: fixed
Status: newclosed

The system check framework's SILENCED_SYSTEM_CHECKS setting allows you to ignore model validation errors and allow runserver to proceed.

comment:12 by Anders Hovmöller, 3 years ago

This ticket was never fixed. The SILENCED_SYSTEM_CHECKS feature doesn't do what you would think it does based on this discussion, and even if it did work as the documentation says (it doesn't!) it would still be no good:

  1. This ticket asked for the runserver to not die when there are validation errors. This request was totally ignored, yet the ticket was set to "fixed".
  2. SILENCED_SYSTEM_CHECKS is a list of strings for what to disable. But what strings are valid? If you put something invalid here it's ignored. You have to step through the code I guess.
  3. Model validation checks aren't even in the SILENCED_SYSTEM_CHECKS list from what I can tell.
  4. SILENCED_SYSTEM_CHECKS only silences the OUTPUT of the checks. If they fail hard it still fails hard.

in reply to:  12 comment:13 by Brendan Jurd, 3 years ago

Replying to boxed:

This ticket was never fixed. The SILENCED_SYSTEM_CHECKS feature doesn't do what you would think it does based on this discussion, and even if it did work as the documentation says (it doesn't!) it would still be no good:

Wow, OP here. thanks for the reply, boxed. I certainly was surprised to see this old thing pop up in my inbox.

I agree with your assessment that the original request was never satisfied, and that this ticket was closed improperly. I didn't get too worked up about that at the time, because I'd already found a way to work around the particular issue I was having.

I think my original point here is still valid, I still believe that runserver should proceed to run even if model validation fails, or at least, there should be an option to enable that behaviour.

Having said that, in the eight years (yikes) since I wrote the original ticket, I've long since stopped trying to coerce the Django ORM into doing anything beyond the basics. These days I just bomb straight out to raw SQL for anything even slightly exotic. There's a lot to like about Django, but its ORM (really, ORMs in general) is just a big old energy drain for me. It took me a while to realise this. SQL is fine. It was fine all along.

So TL;DR: you're right, but I am no longer invested.

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