-
Notifications
You must be signed in to change notification settings - Fork 150
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
Move orjson to be an optional dependency #516
Move orjson to be an optional dependency #516
Conversation
Use it if available for faster parsing but for those less keen on using a multi stage docker build and ensuring they have the correct shared library dependencies (particularly challenging if building off an alpine container that doesn't use a distribution build) they can avoid the extra_requirement.
"faster_parsing": ["orjson"], | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh didn't even know we could use it like this!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We also need to update the documentation along with this PR.
I've added a few notes to the documentation - let me know if I'm on the right track with the kind of update to the documentation you are thinking or if you were thinking of something else? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this all looks great. Thanks for the PR ! 🎉
Use it if available for faster parsing but for those less keen on using
a multi stage docker build and ensuring they have the correct shared
library dependencies (particularly challenging if building off an alpine
container that doesn't use a distribution build) they can avoid the
extra_requirement.