#11716: Various methods in django.db.models.fields don't wrap ValueErrors and
allow
them to escape
---------------------------------------------------+------------------------
Reporter: Leo | Owner: Leo
Status: assigned | Milestone: 1.2
Component: Database layer (models, ORM) | Version: SVN
Resolution: | Keywords:
Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 0
Needs_better_patch: 0 |
---------------------------------------------------+------------------------
Comment (by Leo):
Replying to [comment:15 kmtracey]:
> I'm saying it's policy not to introduce backwards-incompatible changes
in a micro or minor release update, and therefore I have concerns about
this patch, because it seems to me it may likely cause previously robust
application code to start raising exceptions.
I agree that it will cause some applications to start raising exceptions.
I think your concerns are vary valid and critical to consider for a
project like Django. I believe that its a better choice for Django to make
this change in this release and take the impact of introducing this
behavior change especially since non-trivial parts of the DB architecture
have been worked on for this release (including the very functions this
patch changes).
> You say the consequences of not fixing this bug are significant. What
I'm saying is we cannot now break code in the wild that has already been
written to accommodate those consequences.
Sure, but that's true for virtually any bugfix so this isn't an absolute.
This is a discussion with a range of impacts of either making the change
or not making it.
> This code catches !ValueError there because that is the exception that
has in fact been raised in observed cases where bad data is present in
`request.GET['c']`.
> It doesn't catch !ValidationError because that has not been an exception
observed to be raised.
In my experience the approach of seeing what exceptions I can observe has
not been a great way figure out what exceptions to catch in the code that
I write. I've found that fairly often I either overlook a potential
failure case or do not fully understand (or sometimes don't even have
access to) how the code I'm working with behaves. This has resulted in me
preferring to think about interfaces and how the code should behave and
defining those interfaces in one form or another.
> Now, it may well be possible to cause a !ValidationError here, since a
custom comments model may have a custom pk field of some non-standard
type, a type that raises !ValidationError on being handed invalid data.
But for a typical auto-generated pk field, I cannot find any case where a
querystring parameter value (which will be a unicode string) containing
'bad' data generates anything other than a !ValueError. (Based on prior
experience I thought !TypeError was another possibility but I can't even
get that one to be raised.)
It doesn't need to be a very non-standard field. A `DecimalField` will
cause the problem. It's a slightly larger test case to create a custom
`Comments` model but I've attached a very simple one that breaks another
part of the comments application that also doesn't catch `ValidationError`
properly.
> So my concern is that there may be Django users with apps with code
written much like this, and I do not think they will be pleased if we now
change what gets raised there from a !ValueError to a !ValidationError.
That change will cause their code, which previously gracefully handled
being handed bad data values, to now start returning server error pages. I
don't think they will see that change as an improvement.
I think you're right. Very few developers are happy when their code
doesn't work as they expect. In my experience the best way to prevent that
is to have interfaces on commonly used components. If this change is
properly messaged in the release notes I believe that it will make Django
users happy when they can replace rather ugly and patched code with much
cleaner code that represents a contract and gives them more assurance that
their app won't break.
I respect your concerns about this patch and I think we're both striving
to make Django better but I suspect we may not come to agreement on this
issue so I'm wondering what the right next step is given Django's process.
Should this be brought up for discussion on django-dev? is there a vote by
the core committees? does the release manager decide this?
--
Ticket URL: <http://code.djangoproject.com/ticket/11716#comment:16>
Django <http://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To post to this group, send email to django-upda...@googlegroups.com.
To unsubscribe from this group, send email to
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/django-updates?hl=en.
