Skip to content

kafka: kafka client zstd compression #7

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

Merged
merged 2 commits into from
Mar 19, 2020

Conversation

gabriel-tincu
Copy link

@gabriel-tincu gabriel-tincu commented Mar 18, 2020

For internal review

Add zstd compression, tests for it, and update the API objects for the produce req/resp and fetch req/resp , since the minimal version required for handling zstd compressed requests uses a different api.

@gabriel-tincu gabriel-tincu force-pushed the gabi-kafka-client-zstd-compression branch 3 times, most recently from 8af0f81 to 4ca9a84 Compare March 18, 2020 10:37
@gabriel-tincu gabriel-tincu force-pushed the gabi-kafka-client-zstd-compression branch from 4ca9a84 to 150c8b3 Compare March 18, 2020 18:16
@gabriel-tincu gabriel-tincu requested a review from jappja March 19, 2020 11:25
Copy link

@jappja jappja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM for the zstd parts, someone with better Kafka protocol knowledge should also review.

@ivanyu Could you review too?

@ivanyu ivanyu self-assigned this Mar 19, 2020
Copy link
Member

@ivanyu ivanyu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of minor things, but overall LGTM 👍

…ependency file, readme and codec checks

add 2.1.0 to possible releases being tested / downloadable, skip zstd tests when version is too old
add zstd tests to producer tests, add zstandard to tox dep list
add New produce request/response classes to support up to V8 produce,(V7 is required for zstd compression)
add ProduceResponse fields in use after V5
force broker version onto producers on ver 2.4.0 in unittests, due to inconsitencies with the env var for it
produce response api V2 included in the same logic branch as 3 and 5
add proper docstrings to produce request / response classes
impose limit on decompress output size on a failed decompress attemtp
update producer tests to be more idiomatic with the testing framework
@gabriel-tincu gabriel-tincu force-pushed the gabi-kafka-client-zstd-compression branch from 150c8b3 to cc3c3fd Compare March 19, 2020 15:32
@gabriel-tincu gabriel-tincu requested a review from ivanyu March 19, 2020 15:37
@ivanyu ivanyu merged commit 0197209 into master Mar 19, 2020
@Yensan
Copy link

Yensan commented Jul 5, 2022

hi, guys in aiven. There is an enhancement issue in upstream: dpkp#1396
aiven is a data tech compony, using kafka frequently. Do you fix this issue in your fork?
Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants