Increase topic partition count API #533
Conversation
lib/kafka/client.rb
Outdated
| # the topic | ||
| # @param timeout [Integer] a duration of time to wait for the new | ||
| # @param validate_only [Boolean] whether this request is to validate only | ||
| # without actually execute it |
There was a problem hiding this comment.
I don't see any use cases for validate_only in ruby-kafka – do you have any? Otherwise I think it's better to leave it out (and just pass false).
lib/kafka/client.rb
Outdated
| # @param version [Integer] API version. | ||
| # @return [Boolean] | ||
| def support_api?(api_key, version = nil) | ||
| @cluster.support_api?(api_key, version) |
There was a problem hiding this comment.
Can you rename support_api? to supports_api??
Actually, I'm not sure this method should be exposed at all. I don't think the client should deal with this – a method should simply raise an exception if the Kafka version doesn't support the APIs it needs.
Back to the original: can you rename support_api? to supports_api??
lib/kafka/protocol/encoder.rb
Outdated
| @@ -30,7 +30,8 @@ def write(bytes) | |||
| # @param boolean [Boolean] | |||
| # @return [nil] | |||
| def write_boolean(boolean) | |||
| write(boolean ? 0x1 : 0x0) | |||
| # Note: 0x1 is represented by 1 byte when being written into the socket | |||
| write(boolean ? write_int8(1) : write_int8(0)) | |||
There was a problem hiding this comment.
This seems wrong – you're writing the result of write_int8.
|
|
||
| example "create partitions" do | ||
| unless kafka.support_api?(Kafka::Protocol::CREATE_PARTITIONS_API) | ||
| skip("This Kafka version not support ") |
There was a problem hiding this comment.
Ah, now I see why it might be handy to include support_api?.
|
@dasch Updated as your comment. About the |
|
@nguyenquangminh0711 in that case, shouldn't we just do |
|
@dasch Oh you are right. The outer |
|
Thanks! |
Proposed usage
This API is available after Kafka 1.0.0. The API is implemented based on official documentation. Currently, the interface doesn't support custom partition assignments because I don't think it aligns with the goal of the gem.