Fix PydanticKnownError user context handling issues #839
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #839 +/- ##
==========================================
+ Coverage 93.52% 94.04% +0.51%
==========================================
Files 102 102
Lines 14609 15024 +415
Branches 25 25
==========================================
+ Hits 13663 14129 +466
+ Misses 940 889 -51
Partials 6 6
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
CodSpeed Performance ReportMerging #839 will degrade performances by 48.57%Comparing Summary
Benchmarks breakdown
|
bd51836 to
c9a653e
Compare
|
please review |
|
Given the performance consequences, I'm going to defer to one of the rust/pydantic-core experts @samuelcolvin @adriangb @davidhewitt |
| @@ -332,7 +394,7 @@ def test_error_decimal(): | |||
| e = PydanticKnownError('greater_than', {'gt': Decimal('42.1')}) | |||
| assert e.message() == 'Input should be greater than 42.1' | |||
| assert e.type == 'greater_than' | |||
| assert e.context == {'gt': 42.1} | |||
| assert e.context == {'gt': Decimal('42.1')} | |||
There was a problem hiding this comment.
As a bonus this issue also gets fixed
26025a3 to
6c3eb23
Compare
6c3eb23 to
b723ca3
Compare
b723ca3 to
460b4a6
Compare
There was a problem hiding this comment.
This change looks good to me, many thanks.
The performance numbers look like there may be a little bit of (expected) slowdown on error pathways but also a lot of that is just noise from performance instabilities we've had on codspeed recently.
I also defend this change as a correction to behaviour so think we should merge this and if we need to optimize later, that's a problem for the future.
Change Summary
Fixes error context issues with
PydanticKnownErrorwhich lost user provided error context data and caused types to change from what was given to the context.Fixing this requires passing error context data as a part of the
ErrorTypeenum. It is not enough to add the context data as a part of thePydanticKnownErroras internally this is converted intoErrorTypeenum. This conversion was lossy for the context data. Now this issue no longer happens. Adding the context as a part of theErrorTypehappens via a macro that creates the Enum items. Also creating the utilities for using them and handling the error context data. Basic errors without any predefined fields are used viaErrorTypeDefaults::TheErrorconstants to make it easier to use those (without having to addcontext: Nonein cases where error is internally used).Related issue number
fix #824
Checklist
pydantic-core(except for expected changes)Selected Reviewer: @dmontagu