Added ability to set max_request_size and max_partition_fetch_bytes.

maki-nage / makinage

Stream Processing Made Easy

https://www.makinage.org

MIT License

38 stars 1 forks source link

Added ability to set max_request_size and max_partition_fetch_bytes. #10

Closed freemo closed 2 years ago

MainRo commented 2 years ago

Thanks for the proposal. This is a welcomed addition. I have some comments here:

you used the "application" section for the new configuration. Since this is specific to kafka, it should be in the "kafka" section.
Why did you set 512000000 as a default value? aiokafka uses 1048576 by default, so we should use this to keep the existing behavior.
The max_partition_fetch_bytes parameter is exposed in cyclotron-aiokafka but not propagated internally up to aiokafka. Do you have an associated pull request on cyclotron-aiokafka?

freemo commented 2 years ago

First two points I agree with. Third point I need to look a bit closer and get back to you, perhaps I made a mistake.

freemo commented 2 years ago

@MainRo The first two points have now been fixed and are sitting in this PR ready for you to merge

To your third point, it appears cyclotron-aiokafka already supports the argument. See line 29 here: https://github.com/MainRo/cyclotron-aiokafka/blob/master/cyclotron_aiokafka/kafka.py#L29

freemo commented 2 years ago

@MainRo Any updates?

MainRo commented 2 years ago

I did not have the time to merge it yet. It should be done in the coming days. I need to release cyclotron-aiokafka before because the argument is not propagated up to aiokafka (the parameter from Consumer is never used)

MainRo commented 2 years ago

I updated cyclotron-aiokafka. Thanks!