add CreatePartitionsRequest/Response #985
Conversation
buyology
changed the title
|
sorry for the multiple pushes. realised — while writing up some of the other new request/response pairs (I have all of the re: travis — seems to be related to the fact that the Apache mirror only has |
| @@ -44,6 +44,10 @@ func (pe *prepEncoder) putArrayLength(in int) error { | |||
| return nil | |||
| } | |||
|
|
|||
| func (pe *prepEncoder) putBool(in bool) { | |||
| pe.length++ | |||
There was a problem hiding this comment.
might be tempted to just delegate this to putInt8 since that's the effective implementation elsewhere
There was a problem hiding this comment.
the above is mostly for symmetry :) bool is quite common in the new request/responses + just thinking it's nice to have getBool() sanity check for valid values. what do you think. is it ok to leave in?
real_decoder.go
Outdated
| tmp, err := rd.getInt16() | ||
| if err != nil || tmp == -1 { | ||
| str, err := rd.getString() | ||
| if err != nil || str == "" { |
There was a problem hiding this comment.
I think the whole point of nullable strings was to distinguish between a length of -1 (null) and a length of 0 ("") which this check doesn't do, because getString already turns them both into "".
There was a problem hiding this comment.
I factored out getStringLength() now to allow for exactly this logic. the problem with the former getNullableString() impl was that it consumed the length so when calling out to getString() the output became corrupted.
create_partitions_request.go
Outdated
| return nil | ||
| } | ||
|
|
||
| pe.putInt32(int32(len(t.Assignment))) |
| } | ||
|
|
||
| type TopicPartition struct { | ||
| Count int32 |
There was a problem hiding this comment.
I'm looking at https://kafka.apache.org/protocol.html#The_Messages_CreateTopics and this doesn't seem quite right. Aren't you missing the number of replication factor?
There was a problem hiding this comment.
Hm. This is actually https://kafka.apache.org/protocol#The_Messages_CreatePartitions, so it's only about adding about new partitions to an already existing topic.
|
Thanks! |
this adds the request/response pair for
CreatePartitions