-
Notifications
You must be signed in to change notification settings - Fork 13.5k
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
[FLINK-37184][connector/filesystem] Add ZStandard to supported standard decompressors #26029
base: master
Are you sure you want to change the base?
Conversation
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.
2 thoughts:
- I think there should be a unit test for this
- It would be useful to document these decompressors and how this effects the API.
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.
Please can you supply a unit test to that makes use of this change.
Reviewed by Chi on 23/01/2025 Go back to the submitter with review comments. |
@davidradl Thanks for looking at my PR. I'll add some sort of unit test tomorrow, if you have anything specific in mind, please let me know. On the documentation I wholeheartedly agree. But I am pretty new to Flink and I don't know how to change the documentation or really even what to document. I'm still a little bit confused about the status of https://cwiki.apache.org/confluence/display/FLINK/FLIP-27%3A+Refactor+Source+Interface. It's been quite a while, but the old API has better support for compressions: it has zstd added and it has an API to register more compressions for Flink users, while the new API has neither. |
@davidradl I added an unit test |
…rd decompressors This was missed when ZStandard was first implemented.
What is the purpose of the change
Brief change log
Verifying this change
Does this pull request potentially affect one of the following parts:
@Public(Evolving)
: noDocumentation