MINOR: Cleanups in Tools Module (3/n)#20332
Conversation
|
Adding @chia7712 as he kindly helped with all PR reviews in this region. |
sjhajharia
changed the title
|
A label of 'needs-attention' was automatically added to this PR in order to raise the |
chia7712
left a comment
chia7712
left a comment
There was a problem hiding this comment.
@sjhajharia thanks for your patch
| PrintStream err) { | ||
|
|
||
| @Override | ||
| public String toString() { |
There was a problem hiding this comment.
how about trusting the generated toString?
There was a problem hiding this comment.
The auto generated one will also have the out and err params which the record takes in. Hence I prefered keeping this custom one.
| this.topicPartition = topicPartition; | ||
| this.producerState = producerState; | ||
| } | ||
| private record OpenTransaction(TopicPartition topicPartition, ProducerState producerState) { |
There was a problem hiding this comment.
Based on the use cases, using Map<TopicPartition, ProducerState> seems more suitable.
There was a problem hiding this comment.
Sorry, but I dont cpmpletely follow here. Do we want to change the way OpenTransaction is defined?
If so, can we create a ticket and deal with the same separately as it may cause some additional changes in this file.
There was a problem hiding this comment.
yes, I will create a follow-up for it
|
|
||
| private record PluginLocation(Path path) { | ||
| @Override | ||
| public String toString() { |
There was a problem hiding this comment.
I don't think we can get rid of this toString() method.
The default Java toString would create a value like "PluginLocation[path=...]", but the expectation here is to just have the path and not the name of the class.
This PR aims at cleaning up the tools module further by getting rid of some extra code which can be replaced by `record` Reviewers: Chia-Ping Tsai <chia7712@gmail.com>
* Revert "Merge pull request #161 from showuon/cl3_ut" This reverts commit 55efc8d, reversing changes made to 1c8b6d3. * Revert "Merge pull request #155 from showuon/cl3_share_auto_create" This reverts commit 05f8874, reversing changes made to 143e6cc. * MINOR: Cleanups in Tools Module (3/n) (apache#20332) This PR aims at cleaning up the tools module further by getting rid of some extra code which can be replaced by `record` Reviewers: Chia-Ping Tsai <chia7712@gmail.com> * MINOR: kafka-stream-groups.sh should fail quickly if the partition leader is unavailable (apache#20271) This PR applies the same partition leader check for `StreamsGroupCommand` as `ShareGroupCommand` and `ConsumerGroupCommand` to avoid the command execution timeout. Reviewers: Lucas Brutschy <lucasbru@apache.org> * KAFKA-19662: Allow resetting offset for unsubscribed topic in kafka-share-groups.sh (apache#20453) The `kafka-share-groups.sh` tool checks whether a topic already has a start-offset in the share group when resetting offsets. This is not necessary. By removing the check, it is possible to set a start offset for a topic which has not yet but will be subscribed in the future, thus initialising the consumption point. There is still a small piece of outstanding work to do with resetting the offset for a non-existent group which should also create the group. A subsequent PR will be used to address that. Reviewers: Jimmy Wang <48462172+JimmyWang6@users.noreply.github.com>, Lan Ding <isDing_L@163.com>, Apoorv Mittal <apoorvmittal10@gmail.com> * KAFKA-19662: Reset share group offsets for unsubscribed topics (apache#20708) This PR allows the kafka-share-groups.sh --reset-offsets tool to be used to set offsets for topics which are not currently subscribed in a share group. It also works if the share group does not yet exist. This brings the capability in line with the equivalent function in Kafka-consumer-groups.sh. The primary purpose is to allow offsets to be set before the share group is first used as a way of initialising in a known state. Reviewers: Jimmy Wang <48462172+JimmyWang6@users.noreply.github.com>, Kuan-Po Tseng <brandboat@gmail.com>, Apoorv Mittal <apoorvmittal10@gmail.com> --------- Co-authored-by: Sanskar Jhajharia <sjhajharia@confluent.io> Co-authored-by: jimmy <wangzhiwang611@gmail.com> Co-authored-by: Andrew Schofield <aschofield@confluent.io>
2 participants
This PR aims at cleaning up the tools module further by getting rid of
some extra code which can be replaced by
recordReviewers: Chia-Ping Tsai chia7712@gmail.com