Use fastjsonschema for json schema validation #64
Conversation
|
@kiendang, are you still working on this PR? This looks good to me. Great work here! |
| json_validator = CaselessStrEnum( | ||
| JSON_SCHEMA_VALIDATORS.keys(), | ||
| 'fastjsonschema', | ||
| help=""" | ||
| JSON schema validation implementation. | ||
|
|
||
| Valid choices are `jsonschema` and `fastjsonschema`. | ||
| """ | ||
| ).tag(allowed_none=False) |
There was a problem hiding this comment.
@Zsailer Should we allow users to subclass here? They might want to customize the default validator or use their own. Or we can compromise and allow them to past a class name but restricted to only JSONSchemaValidator and FastJSONSchemaValidator and later can consider passing any class if someone asks about that use case. Is there any downside to allowing subclassing?
There was a problem hiding this comment.
Good question. I don’t see any downside to allowing custom validators by subclassing EventSchemaValidator. We can make this trait a Type(default_value='FastJSONSchemaValidator', klass='EventSchemaValidator').
I'm done for now except for that one comment. But I converted to draft again since I want to think a bit about how this integrates with the category filtering, especially in the future where draft-2019/draft-2020 is supported. |
|
Great. Ping me here when you’re ready for a final review. |
|
Sorry for not getting back on this sooner.
My proposal is we defer making decision on this for a bit longer since this is "only" a performance thing. Let me look into |
|
I do have a WIP on that. Will find time to finish it somewhere next week. I'd love to see telemetry integrated in too. |
|
I wanted to chime in and say that another reason to be interested in fastjsonschema is that it removes a C dependency! jsonschema now depends on https://pypi.org/project/pyrsistent/ which is not pure python, and since nbformat and friends use it the jupyter stack is not easily installable on pyiodide now |
Add default option to use fastjsonschema to validate events. fastjsonschema uses code gen and thus should speed up validation since we don't need to traverse the schema again every time we validate an event, and the event json traversal/validation would be a lot faster too. Will add benchmarks soon.
This is relevant for #59 where there is performance concern about running the validation twice.
Inspired by
https://github.com/jupyter/nbformat/blob/dd4725fb41515c69bc76e1c7f32f3ede0075725f/nbformat/json_compat.py