-
Notifications
You must be signed in to change notification settings - Fork 126
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
Add zstd pre-and-postprocessor. #1101
Conversation
The code required seems too straightforward. I feel I am missing some stuff. Also I should probably add some tests, but I do not know what tests. |
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.
Heya :) awesome! thank you! only a few small things, one I commented inline, then there should be some tests for the processors and the lookup. You can find examples on how to dot hat in the preprocessors.rs
there the other compression algorithms are tested too.
The https://github.com/tremor-rs/tremor-www-docs/blob/main/docs/artefacts/postprocessors.md and https://github.com/tremor-rs/tremor-www-docs/blob/main/docs/artefacts/preprocessors.md will need updating to reflect the new processor.
Otherwise 👍 great job :D
That was quick! 😎 🚀 |
We might also need to add libzstd as dependencies to our rpm, deb packages and to the docker image. |
@mfelsche Where should I update this dependency for the build. |
Signed-off-by: Daksh Chauhan <dak-x@outlook.com>
Signed-off-by: Daksh Chauhan <dak-x@outlook.com>
I think zstd brings zstd-sys which is bringing its own |
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.
Woooh thank you!! 🚀
Signed-off-by: Daksh Chauhan dak-x@outlook.com
Pull request
Add
zstd
pre-and-postprocessorsDescription
Related
zstd
pre- and postprocessors #1100Checklist
Performance