-
-
Notifications
You must be signed in to change notification settings - Fork 68
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature request: make "context in exceptions" optional #187
Comments
That is an exciting idea. I will measure how it changes the performance, and if it's improving it significantly, I will provide an option for that. |
A quick check shows it could reduce time by 50%! For cases when you expect mostly valid inputs and you don't care about details, it could speed up greatly the performance. Will provide this feature for sure. :-) |
Added |
We don't use fastjsonschema's exception info, falling back to jsonschema for that. Not using the info is faster. Possible because horejsek/python-fastjsonschema#187 was completed. I did not at present do any speed/memory checks, but the diff is quite promising :-)
I'm using fjs to generate python code that does the validation (i.e.
compile_to_code
)I'm working around #72 by just falling back to good (bad) old
jsonschema
andbest_guess
in the case I have a validationerror.That means I don't use any of the information that's present in the
JsonSchemaValueException
. Looking at that code, I noticed that ~75% (out of a whopping 2.5MB) is used by thedefinition
parts of the generated exceptions. When you have deeply nested definitions, that is to be expected, since the relevant part of the definition (for some part) is repeated throughout the tree.Since I don't use that information, generating it (and loading it) seems wasteful. It would be nice if
compile_to_code
had a parameter that controlled the behavior of the generatedraise JsonSchemaValueException
, basically making (the relevant parts of) this commit conditional.I managed to work around this a bit by hacking around in
def exc
but that doesn't "quite feel right".The text was updated successfully, but these errors were encountered: