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
> >>
>

Reply via email to