Opened 3 years ago

Closed 13 months ago

#19126 closed New feature (fixed)

Allow runserver to bypass model validation

Reported by: direvus Owned by: nobody
Component: Core (Management commands) Version: master
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 direvus 3 years ago.
Patch to catch and report validation errors in runserver.

Download all attachments as: .zip

Change History (12)

Changed 3 years ago by direvus

Patch to catch and report validation errors in runserver.

comment:1 follow-up: Changed 3 years ago by lrekucki

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

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.

comment:2 in reply to: ↑ 1 Changed 3 years ago by direvus

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 Changed 3 years ago by aaugustin

  • Triage Stage changed from Unreviewed to Design decision needed
  • Type changed from Uncategorized to Bug

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 Changed 2 years ago by anonymous

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 Changed 2 years ago by kmtracey

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 follow-up: Changed 2 years ago by aaugustin

  • Triage Stage changed from Design decision needed to Accepted

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.

comment:7 in reply to: ↑ 6 Changed 2 years ago by direvus@…

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 follow-up: Changed 2 years ago by aaugustin

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

comment:9 in reply to: ↑ 8 Changed 2 years ago by direvus

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

  • Summary changed from runserver versus model validation to Allow runserver to bypass model validation
  • Type changed from Bug to New feature
  • Version changed from 1.3 to master

comment:11 Changed 13 months ago by timo

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

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

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