search

netzkolchose / django-fast-update

Faster db updates using UPDATE FROM VALUES sql variants.
MIT License
21 stars 2 forks source link

Encoders error messages should include info of what type an object is when it's not the expected one #2

Closed mikicz closed 2 years ago

mikicz commented 2 years ago

A lot of my tests are failing because of TypeError coming from the encoders, and the error message is not very helpful.

  File "[REDACTED].venv/lib64/python3.9/site-packages/fast_update/copy.py", line 90, in Int
    raise TypeError(f'expected type {int} was {type(v)}')
TypeError: expected type <class 'int'>

I think it would make debugging much easier if it said what type the object was. For example:

  File "[REDACTED].venv/lib64/python3.9/site-packages/fast_update/copy.py", line 90, in Int
    raise TypeError(f'expected type {int} was {type(v)}')
TypeError: expected type <class 'int'> was <class 'NoneType'>

Best would of course be if it also said what field the issue is in, but that might be harder to do.

jerch commented 2 years ago

Yeah good point, gonna make this more explicit. As you already said, field or even row/obj backtracking will be alot harder, as the code currently tries very hard to process things fast. Will see what I can do about that.

jerch commented 2 years ago

@mikicz Giving the field name as argument to the encoders has only small perf penalty (<<5%), so gonna add that for easier debugging.

Exception message would look like:

...
    raise TypeError(f'expected type {int} for field "{fname}", got {type(v)}')
TypeError: expected type <class 'int'> for field "f1", got <class 'str'>

Edit: Also tested with additional pk or position reporting, this makes it way more expensive adding ~15% to total runtime. Thus I wont add it for now. Imho the fieldname and the wrong type are enough clues for anyone to scan their data for the culprits.