Build the partitionConsumer into the topicConsumer #442
Conversation
| return nil, err | ||
| } | ||
| pList = append(pList, int32(val)) | ||
| } |
There was a problem hiding this comment.
Should we check whether the partition is the c.Partitions(*topic) list, or just let it fail later?
There was a problem hiding this comment.
It fails later pretty nicely with ERROR: Failed to start consumer for partition -1: kafka server: Request was for a topic or partition that does not exist on this broker.
There was a problem hiding this comment.
👍 that should be enough :)
|
Maybe just call it |
|
Oh, does this whole thing break semantic versioning? Does this need to be a 2.0 change? |
|
I don't feel it's needed, but maybe we should state this in the |
|
Added a note to |
|
The alternative would just be to not remove |
|
Like I said, I don't feel too strongly about maintaining backwards compatibility for the tools, but if you do that works for me 👍 |
Add a flag to support arbitrary partitions. Deprecate (but do not remove, yet) partitionConsumer tool.
|
I'm not 100% sure I want to commit to API stability for the tools for eternity, but in this case it's easy enough. I'll add "remove partitionConsumer" to our 2.0 wiki page. |
|
Last 👀? I had to do some git dancing to rebase while undeleting the partitionConsumer. |
| - [kafka-console-partitionconsumer](./kafka-console-partitionconsumer): a command line tool to consume a single partition of a topic on your Kafka cluster. | ||
| - [kafka-console-topicconsumer](./kafka-console-topicconsumer): a command line tool to consume all partition of a topic on your Kafka cluster. | ||
| - [kafka-console-partitionconsumer](./kafka-console-partitionconsumer): (deprecated) a command line tool to consume a single partition of a topic on your Kafka cluster. | ||
| - [kafka-console-consumer](./kafka-console-consumer): a command line tool to consume arbitrary partitions of a topic on your Kafka cluster. |
There was a problem hiding this comment.
"all or an arbitrary list of"?
There was a problem hiding this comment.
"all of them" still qualifies as an arbitrary list :)
|
minor wording quibble - |
Build the partitionConsumer into the topicConsumer
@Shopify/kafka this is what I meant by merging them. Adding a flag to the topicConsumer makes the partitionConsumer obsolete.