KIP-339: Add Incremental Config updates API #1966
Conversation
dnwe
changed the title
|
Hello @dnwe! Fixing tests - response in case of AlterConfigs and IncrementalAlterConfigs is the same, thats why tests passed. Best Regards, |
| // IncrementalAlterConfigsResponse is a response type for incremental alter config | ||
| type IncrementalAlterConfigsResponse struct { | ||
| ThrottleTime time.Duration | ||
| Resources []*AlterConfigsResourceResponse |
There was a problem hiding this comment.
In this place I have used AlterConfigsResourceResponse - thats why file with according definition is also affected. In same way it is used in Kafka Java Client.
There was a problem hiding this comment.
@ajanikow welcome and thanks for another great PR ⭐
I've reviewed the changes and I'm happy that the protocol matches with the V0 types for request and response as defined in the protocol spec. Thanks for adding test cases to cover the roundtripping of the request and response types.
Lets go ahead and merge this PR now so that people can start consuming it. If you have some time, there's a couple of interesting follow-up PRs that would be great if you could also take on.
It would be good to add something to admin.go to expose the per-broker IncrementalAlterConfigs func call in the same way as we currently expose the regular one via AlterConfig in admin.go
https://github.com/Shopify/sarama/blob/d01b3443aa926c7a04f00182002e77fea420ac57/admin.go#L666-L716
I also wondered if you might be able to add a functional test (see the various functional_ prefixed files) that drives this API against a real Kafka cluster so we can be confident that the functionality is working and doesn't regress in the future
|
Hello! For integration tests it is not a problem, already have them in my project. This is very useful in case of Broker Dynamic Configuration (can be tested to, for example, add listener in runtime) Will add them together with admin.go modifications. Best Regards, |
|
Hello!
In this PR I cover IncrementalAlterConfigs API in Sarama. It is very useful, due to fact we can specify which config entry needs to be affected (instead of sending everything over and over again).
Fixes: #1830, birdayz/kaf#155
Kafka Code Links:
Best Regards,
Adam.