[Django] #34557: Time-based model field cleaning and TypeErrors
Django
-------------------------------------+-------------------------------------
Reporter: Claude | Owner: nobody
Paroz |
Type: | Status: new
Cleanup/optimization |
Component: Database | Version: dev
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
In the case I set a wrong integer value to some time-based field (say a
year as integer) and call `full_clean` on the instance, I'll get a
`TypeError` as only `ValueError`s are catched to produce
`ValidationError`s.
Should that use case be supported and should `Date(Time)Field.to_python`
also catch `TypeError`s?
--
Ticket URL: <https://code.djangoproject.com/ticket/34557>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Type: | Status: closed
Cleanup/optimization |
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution: duplicate
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => duplicate
Comment:
This is probably a duplicate of #21523, see
[https://code.djangoproject.com/ticket/21523#comment:17 comment]:
> The real question is the contract of the to_python method:
>
> - Should it accept anything and never crash?
--
Ticket URL: <https://code.djangoproject.com/ticket/34557#comment:1>
Django
-------------------------------------+-------------------------------------
Reporter: Claude Paroz | Owner: nobody
Type: | Status: closed
Cleanup/optimization |
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution: duplicate
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Claude Paroz):
Yes, this is partially related to #21523, thanks for pointing it out. I
checked the `to_python()` method of other model fields as well:
* The default `to_python` is just returning the passed value. The
docstring say: `raising django.core.exceptions.ValidationError if the data
can't be converted` which is likely the case in this ticket's use case.
* `BooleanField.to_python` is making some `if` tests, then raises
`ValidationError` for any other values.
* `CharField.to_python`/`TextField.to_python` are casting to string every
non-`str` value.
* `DecimalField.to_python` is catching `TypeError` when trying to cast to
`Decimal`.
* `FloatField.to_python` is catching `TypeError` when trying to cast to
`float`.
* `IntegerField.to_python` is catching `TypeError` when trying to cast to
`int`.
* `GenericIPAddressField.to_python` is casting values to `str` (aka
`CharField`).
* `BinaryField.to_python` is just returning the value if not a `str`.
* `UUIDField.to_python` is catching `AttributeError` which will be
returned for any non-supported types (because of calling `.replace` on the
value).
In the custom model fields documentation, we can read: `For to_python(),
if anything goes wrong during value conversion, you should raise a
ValidationError exception.`
So after reviewing different `to_python` implementations, there is clearly
a discrepancy between date-related fields (including `DurationField`) and
all other model fields. And whatever I said in the past, today I would
clearly answer Yes to the question of should-not-crash.
I'm still commenting here, as #21523 is a bit different, as solving this
(that is catch `TypeError` would still not solve their use case).
--
Ticket URL: <https://code.djangoproject.com/ticket/34557#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: Claude Paroz | Owner: nobody
Type: | Status: closed
Cleanup/optimization |
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution: duplicate
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak):
Replying to [comment:2 Claude Paroz]:
> I'm still commenting here, as #21523 is a bit different, as solving this
(that is catch `TypeError` would still not solve their use case).
It should solve #21577 which was marked as a duplicate of #21523 :)
--
Ticket URL: <https://code.djangoproject.com/ticket/34557#comment:3>