Hi Joel, Thank you for the KIP.
The KIP is well motivated. I have a couple comments: 1. I would not describe the new option with Java code that you want to add to the `StreamsResetter` class since this class is not part of the public API. Only the script kafka-streams-application-reset.sh in /bin belongs to the public API. I would illustrate the new option similarly as in the output of kafka-streams-application-reset.sh --help. 2. It is not entirely clear to me what you mean by "Do a dry-run to check ..." and "execute without dry-run". Does the user need to execute the script twice? Do you get a prompt to confirm the deletion of the internal topic in the same execution of the script? 3. I would remove "No compatibility issues". The sentence afterward describes how the KIP avoids backward compatibility issues. On a different note: Since it seems that the current behavior can have quite critical consequences on applications -- namely that users might damage other applications without a warning -- I would like to propose to change the script in such a way so that it always makes a dry-run, it asks the users for confirmation, and then resets the app. I do not know if this still counts as backward compatible, though. Any thoughts? Best, Bruno On Fri, Jun 12, 2020 at 9:52 AM Joel Wee Hotmail <joelwe...@hotmail.com> wrote: > > Thanks Boyang, that’s helpful! > > Have made the changes and updated the KIP now. > > Joel > > > On 11 Jun 2020, at 5:38 PM, Boyang Chen <reluctanthero...@gmail.com> wrote: > > > > Thanks for the KIP Joel! Some quick questions and suggestions: > > > > 1. The KIP links to a wrong JIRA, which should be > > https://issues.apache.org/jira/browse/KAFKA-6435 > > 2. Typo: *deletes all topic that *-> *deletes all topics that* > > 3. We need to explain in the Motivation section that we want to implement > > an alternative approach for users to just delete internal topics they would > > like to instead of using app-id-* as the regex > > 4. In the Proposed Changes section, we need to explain the general usage > > pattern, by doing a dry-run first and then choose whether to use the > > `--internal-topics` flag or not. > > 5. Seems we don't need bullet points for the Compatibility section? > > > > Boyang > > > > On Wed, Jun 10, 2020 at 2:16 PM Joel Wee <joel....@outlook.com> wrote: > > > >> Hi all > >> > >> I would like to start the discussion for KIP-623 which proposes adding an > >> “internal-topics” option to the streams application reset tool: > >> > >> https://cwiki.apache.org/confluence/x/YQt4CQ > >> > >> Thanks > >> > >> Joel > >> >
