Add deserialization_exception field for programmatic error handling#316
Add deserialization_exception field for programmatic error handling#316bjester wants to merge 2 commits intolearningequality:release-v0.8.xfrom
Conversation
ozer550
left a comment
ozer550
left a comment
There was a problem hiding this comment.
I might be wrong but it seems like we are doing opposite of "skip rows that previously failed deserialization" ? Was this change in behavior intended? Also adding some tests to verify the null string change for skipping would be useful!
ozer550
left a comment
ozer550
left a comment
There was a problem hiding this comment.
I did a grep on places where we had "deserialization_exception" and found a place where we might need to add the execution path as well?
|
@ozer550 I've updated it to set the exception in the code you found. I also grepped and found some backend code setting to empty string. We probably didn't to set it there, but I updated the code anyway, to be have parity with |
ozer550
left a comment
ozer550
left a comment
There was a problem hiding this comment.
Just a small suggestion not a blocker regarding the Interity Error. Otherwise looks good to me.
| to_pk=to_pk_field.to_python(to_pk), | ||
| ) | ||
| } | ||
| err = IntegrityError( |
There was a problem hiding this comment.
Is there a reason we are specifically choosing IntegrityError here? This is validation issue rather than DB integrity one? Might lead to misinterpretations? Would it be better to have a special dedicated Morango error class?
There was a problem hiding this comment.
Nominally the function "validates", but it validates FKs. The previous implementation was trying to emulate a validation error, but in reality, a missing FK is an integrity issue. Continuing to have it be represented as a validation error, which isn't entirely specific, doesn't benefit the ability to analyze the errors (i.e. new deserialization_exception). So this is an improvement by being more specific, which wasn't really necessary before without deserialization_exception
2 participants
Summary
This allows programmatic handling of deserialization errors instead of parsing error messages
deserialization_exceptionnullable field toStoremodel that stores the fully-qualified exception class path when deserialization failsdeserialization_errorfield to be nullableReferences
Followup to #306
Closes #315
AI Usage
Directed an AI agent to tackle the issue, iterated on its work for a loop, then did a small refactor myself. Had the AI agent add test coverage, then reviewed.