Hi Bruce Thank you for the comments.
I agree with 1 and 3 and have updated the KIP. 2. Yes, the user needs to execute the script twice. Once with `--dry-run` and once without. There are no confirmation prompts when executing without `--dry-run`. I have updated the KIP to make this clearer. It does sound like it would make sense to build the dry-run functionality into the script, but I think that prompting for confirmation will break backwards compatibility – e.g. for anyone who uses the script as part of a fully automated process. I’m not sure if the benefits outweigh the costs here? Best Joel > On 12 Jun 2020, at 4:33 PM, Bruno Cadonna <br...@confluent.io> wrote: > > 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 >>>> >>
