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