Allow impossible model validation (mainly admin and generic forms)

173 views
Skip to first unread message

Vlastimil Zíma

unread,
Nov 25, 2015, 9:23:05 AM11/25/15
to Django developers (Contributions to Django itself)

Django assumes that all input data in forms can be validated by `Form.is_valid()` which in some cases is not true. Database contraints, e.g. unique, can fail even though they are checked. An application may require communication with other servers while processing data which can lead to errors. But these conditions are not expected by regular Django form flow and an attempt to handle these cases results in large overriding of default implementation.


This topic was previously discussed here https://groups.google.com/d/topic/django-developers/rzjpP0byNQo/discussion, but discussion was mainly based on race condition in unique check. I'd like to reopen the topic because there are other possibilities which may cause the possibility of failure after form validation to become real, especially if network connection is involved.


I suggest Django should provide mechanism which allow handling the unexpected failures after form validation, e.g. expect `ValidationError` to be raised by `ModelAdmin.save_model`.


I started this discussion as a result of wontfix from https://code.djangoproject.com/ticket/25777.


Vlastik

Tim Graham

unread,
Nov 25, 2015, 1:57:19 PM11/25/15
to Django developers (Contributions to Django itself)
Would Django itself ever raise ValidationError in Model.save()?

I wonder how you propose converting something like an IntegrityError from the database into an "friendly" exception for the user?

Podrigal, Aron

unread,
Nov 26, 2015, 1:34:32 AM11/26/15
to Django developers (Contributions to Django itself)

I'm also in favor of some solution here. I used to raise a ValidationError with either a particular field if I was able to extract the particular error via regex matches, or as non_field_errors. That was my best I had instead of resulting in a 5xx. In most cases for example a unique race condition, when the user will resubmit the form it will have the detailed validation errors or pass.

--
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/1db10ddc-eae6-4f17-a596-b8ce1aa8ef1f%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Vlastimil Zíma

unread,
Nov 26, 2015, 3:28:19 AM11/26/15
to Django developers (Contributions to Django itself)
Hi guys,

to answer Tim's questions:


> Would Django itself ever raise ValidationError in Model.save()?

That can be possible. Currently I have no clear opinion of where IntegrityError and other errors should be changed to ValidationError.


> I wonder how you propose converting something like an IntegrityError from the database into an "friendly" exception for the user?

The message doesn't have to be specific. Append form's non_field_errors with message like "An unexpected error occured while saving the data. Please try again." is in my opinion way better than server error caused by unhandled exception. I'm not in favor of re based heuristics to get details from integrity error message as noted by Aron.

Vlastik

Dne čtvrtek 26. listopadu 2015 7:34:32 UTC+1 Aron Podrigal napsal(a):

Tom Christie

unread,
Nov 26, 2015, 7:30:50 AM11/26/15
to Django developers (Contributions to Django itself)
> Append form's non_field_errors with message like

Given that the IntegrityError doesn't occur until the point of .save(), it's not obvious how that'd work in terms of the Forms API.
You're absolutely correct that there's a class of constraints that can fail here, but it's not evident to me that dealing with these explicitly when required (in practice, fairly rare) isn't a better tack.

Vlastimil Zíma

unread,
Nov 26, 2015, 9:14:25 AM11/26/15
to Django developers (Contributions to Django itself)
General flow for forms should develop to this:

form = MyModelForm(data)
if form.is_valid():
    try:
        form.save()
    except ValidationError, error:
        form.add_error(None, error)
    else:
        return redirect('success')
return render_failure(form)

While handling such case in custom view is simple. But handling this case, when using views from Django is completely different level. In generic views it is still possible to override `form_valid` to handle IntegrityError or other required exception, but it is not possible to do so in ModelAdmin 'add_view' and 'change_view'. There it requires rebuilding the whole context which is duplication of about 30 lines from 'changeform_view'.

Dne čtvrtek 26. listopadu 2015 13:30:50 UTC+1 Tom Christie napsal(a):
Reply all
Reply to author
Forward
0 new messages