Hey Bruno,

I agree adding a prompt would be a nice precaution, but it is not backward
compatible as you suggested and could make the automation harder to
achieve.

If you want, we may consider starting a separate ticket to discuss whether
adding a prompt to let users be aware of the topics that are about to
delete. However, this is also inverting the assumptions we made about
`--dry-run` mode, which would become useless to me once we are adding a
prompt asking users whether they want to remove these topics completely, or
do nothing at all.

Back to KIP-623, I think this discussion could be held in orthogonal, which
applies to more general considerations of reducing human errors, etc.

Boyang

On Tue, Jun 30, 2020 at 12:55 AM Bruno Cadonna <br...@confluent.io> wrote:

> Hi,
>
> I have already brought this up in the discussion thread.
>
> Should we not run a dry-run in any case to avoid inadvertently
> deleting topics of other applications?
>
> I know it is a backward incompatible change if users use it in
> scripts, but I think it is still worth discussing it. I would to hear
> what committers think about it.
>
> Best,
> Bruno
>
> On Tue, Jun 30, 2020 at 5:33 AM Boyang Chen <reluctanthero...@gmail.com>
> wrote:
> >
> > Thanks John for the great suggestion. +1 for enforcing the prefix check
> for
> > the `--internal-topics` list.
> >
> > On Mon, Jun 29, 2020 at 5:11 PM John Roesler <vvcep...@apache.org>
> wrote:
> >
> > > Oh, I meant to say, if that’s the case, then I’m +1 (binding) also :)
> > >
> > > Thanks again,
> > > John
> > >
> > > On Mon, Jun 29, 2020, at 19:09, John Roesler wrote:
> > > > Thanks for the KIP, Joel!
> > > >
> > > > It seems like a good pattern to document would be to first run with
> > > > —dry-run and without —internal-topics to list all potential topics,
> and
> > > > then to use —internal-topics if needed to limit the internal topics
> to
> > > > delete.
> > > >
> > > > Just to make sure, would we have a sanity check to forbid including
> > > > arbitrary topics? I.e., it seems like —internal-topics should require
> > > > all the topics to be prefixed with the app id. Is that right?
> > > >
> > > > Thanks,
> > > > John
> > > >
> > > > On Mon, Jun 29, 2020, at 18:25, Guozhang Wang wrote:
> > > > > Thanks Joel, the KIP lgtm.
> > > > >
> > > > > A minor suggestion is to explain where users can get the list of
> > > internal
> > > > > topics of a given application, and maybe also add it as part of the
> > > helper
> > > > > scripts, for example via topology description.
> > > > >
> > > > > Overall, I'm +1 as well (binding).
> > > > >
> > > > > Guozhang
> > > > >
> > > > >
> > > > > On Sat, Jun 27, 2020 at 4:33 AM Joel Wee <joel....@outlook.com>
> wrote:
> > > > >
> > > > > > Thanks Boyang, I think what you’ve said makes sense. I’ve made
> the
> > > > > > motivation clearer now:
> > > > > >
> > > > > >
> > > > > > Users may want to specify which internal topics should be
> deleted. At
> > > > > > present, the streams reset tool deletes all topics that start
> with "<
> > > > > > application.id<http://application.id>>-" and there are no
> options to
> > > > > > control it.
> > > > > >
> > > > > > The `--internal-topics` option is especially useful when there
> are
> > > prefix
> > > > > > conflicts between applications, e.g. "app" and "app-v2". In this
> > > case, if
> > > > > > we want to reset "app", the reset tool’s default behaviour will
> > > delete both
> > > > > > the internal topics of "app" and "app-v2" (since both are
> prefixed by
> > > > > > "app-"). With the `--internal-topics` option, we can provide
> > > internal topic
> > > > > > names for "app" and delete the internal topics for "app" without
> > > deleting
> > > > > > the internal topics for "app-v2".
> > > > > >
> > > > > > Best
> > > > > >
> > > > > > Joel
> > > > > >
> > > > > > On 27 Jun 2020, at 2:07 AM, Boyang Chen <
> reluctanthero...@gmail.com
> > > > > > <mailto:reluctanthero...@gmail.com>> wrote:
> > > > > >
> > > > > > Thanks for driving the proposal Joel, I have a minor
> suggestion:  we
> > > should
> > > > > > be more clear about why we introduce this flag, so it would be
> > > better to
> > > > > > also state clearly in the document for the default behavior as
> well,
> > > such
> > > > > > like:
> > > > > >
> > > > > > Comma-separated list of internal topics to be deleted. By
> default,
> > > > > > Streams reset tool will delete all topics prefixed by the
> > > > > > application.id<http://application.id>.
> > > > > >
> > > > > > This flag is useful when you need to keep certain topics intact
> due
> > > to
> > > > > > the prefix conflict with another application (such like "app" vs
> > > > > > "app-v2").
> > > > > >
> > > > > > With provided internal topic names for "app", the reset tool will
> > > only
> > > > > > delete internal topics associated with "app", instead of both
> "app"
> > > > > > and "app-v2".
> > > > > >
> > > > > >
> > > > > > Other than that, +1 from me (binding).
> > > > > >
> > > > > > On Wed, Jun 24, 2020 at 1:19 PM Joel Wee <joel....@outlook.com
> > > <mailto:
> > > > > > joel....@outlook.com>> wrote:
> > > > > >
> > > > > > Apologies. Changing the subject.
> > > > > >
> > > > > > On 24 Jun 2020, at 9:14 PM, Joel Wee <joel....@outlook.com
> <mailto:
> > > > > > joel....@outlook.com><mailto:
> > > > > > joel....@outlook.com<mailto:joel....@outlook.com>>> wrote:
> > > > > >
> > > > > > Hi all
> > > > > >
> > > > > > I would like to start a vote for KIP-623, which adds the option
> > > > > > --internal-topics to the streams-application-reset-tool:
> > > > > >
> > >
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=158862177
> > > > > > .
> > > > > >
> > > > > > Please let me know what you think.
> > > > > >
> > > > > > Best
> > > > > >
> > > > > > Joel
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > >
> > > > > --
> > > > > -- Guozhang
> > > > >
> > > >
> > >
>

Reply via email to