Thanks for the KIP Sang! I have a couple of more comments about the wiki page:
(1) The "Public Interface" section should only list the new stuff. This KIP does not change anything with regard to the existing options `--input-topic` or `--intermediate-topic` and thus it's just "noise" to have them in this section. Only list the new option `allInputTopicsOption`. (2) Don't post code, ie, the implementation of private methods. KIPs should only describe public interface changes. (3) The KIP should describe that we intend to use `describeConsumerGroups` calls to discover the topic names -- atm, it's unclear from the KIP how the new feature actually works. (4) If the new flag is used, we will discover input and intermediate topics. Hence, the name is miss leading. We could call it `--all-user-topics` and explain in the description that "user topics" are input and intermediate topics for this case (in general, also output topics are "user topics" but there is nothing to be done for output topics). Thoughts? -Matthias On 1/27/20 6:35 AM, Sang wn Lee wrote: > thank you John Roesle > > It is a good idea > "—all-input-topics" > > I agree with you > > I'll update right away > > > On 2020/01/24 14:14:17, "John Roesler" <vvcep...@apache.org> wrote: >> Hi all, thanks for the explanation. I was also not sure how the kip would be >> possible to implement. >> >> No that it does seem plausible, my only feedback is that the command line >> option could align better with the existing one. That is, the existing >> option is called “—input-topics”, so it seems like the new one should be >> called “—all-input-topics”. >> >> Thanks, >> John >> >> On Fri, Jan 24, 2020, at 01:42, Boyang Chen wrote: >>> Thanks Sophie for the explanation! I read Sang's PR and basically he did >>> exactly what you proposed (check it here >>> <https://github.com/apache/kafka/pull/7948/files> in case I'm wrong). >>> >>> I think Sophie's response answers Gwen's question already, while in the >>> meantime for a KIP itself we are not required to mention all the internal >>> details about how to make the changes happen (like how to actually get the >>> external topics), considering the change scope is pretty small as well. But >>> again, it would do no harm if we mention it inside Proposed Change session >>> specifically so that people won't get confused about how. >>> >>> >>> On Thu, Jan 23, 2020 at 8:26 PM Sophie Blee-Goldman <sop...@confluent.io> >>> wrote: >>> >>>> Hi all, >>>> >>>> I think what Gwen is trying to ask (correct me if I'm wrong) is how we can >>>> infer which topics are associated with >>>> Streams from the admin client's topic list. I agree that this doesn't seem >>>> possible, since as she pointed out the >>>> topics list (or even description) lacks the specific information we need. >>>> >>>> What we could do instead is use the admin client's >>>> `describeConsumerGroups` API to get the information >>>> on the Streams app's consumer group specifically -- note that the Streams >>>> application.id config is also used >>>> as the consumer group id, so each app forms a group to read from the input >>>> topics. We could compile a list >>>> of these topics just by looking at each member's assignment (and even check >>>> for a StreamsPartitionAssignor >>>> to verify that this is indeed a Streams app group, if we're being >>>> paranoid). >>>> >>>> The reset tool actually already gets the consumer group description, in >>>> order to validate there are no active >>>> consumers in the group. We may as well grab the list of topics from it >>>> while it's there. Or did you have something >>>> else in mind? >>>> >>>> On Sat, Jan 18, 2020 at 6:17 PM Sang wn Lee <ssangdd...@gmail.com> wrote: >>>> >>>>> Thank you >>>>> >>>>> I understand you >>>>> >>>>> 1. admin client has topic list >>>>> 2. applicationId can only have one stream, so It won't be a problem! >>>>> 3. For example, --input-topic [reg] >>>>> Allowing reg solves some inconvenience >>>>> >>>>> >>>>> On 2020/01/18 18:15:23, Gwen Shapira <g...@confluent.io> wrote: >>>>>> I am not sure I follow. Afaik: >>>>>> >>>>>> 1. Topics don't include client ID information >>>>>> 2. Even if you did, the same ID could be used for topics that are not >>>>> Kafka >>>>>> Streams input >>>>>> >>>>>> The regex idea sounds doable, but I'm not sure it solves much? >>>>>> >>>>>> >>>>>> On Sat, Jan 18, 2020, 7:12 AM Sang wn Lee <ssangdd...@gmail.com> >>>> wrote: >>>>>> >>>>>>> Thank you >>>>>>> Gwen Shapira! >>>>>>> We'll add a flag to clear all topics by clientId >>>>>>> It is ‘reset-all-external-topics’ >>>>>>> >>>>>>> I also want to use regex on the input topic flag to clear all >>>> matching >>>>>>> topics. >>>>>>> >>>>>>> On 2020/01/17 19:29:09, Gwen Shapira <g...@confluent.io> wrote: >>>>>>>> Seem like a very nice improvement to me. But I have to admit that I >>>>>>>> don't understand how this will how - how could you infer the input >>>>>>>> topics? >>>>>>>> >>>>>>>> On Thu, Jan 16, 2020 at 10:03 AM Sang wn Lee <ssangdd...@gmail.com >>>>> >>>>>>> wrote: >>>>>>>>> >>>>>>>>> Hello, >>>>>>>>> >>>>>>>>> Starting this thread to discuss KIP-560: >>>>>>>>> wiki link : >>>>>>>>> >>>>>>> >>>>> >>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-560%3A+Auto+infer+external+topic+partitions+in+stream+reset+tool >>>>>>>>> >>>>>>>>> I'm newbie >>>>>>>>> I would like to receive feedback on the following features! >>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> >>>> >>> >>
signature.asc
Description: OpenPGP digital signature
