Re: [DISCUSS] KIP-680: TopologyTestDriver should not require a Properties argument

2020-11-13 Thread Guozhang Wang
Hello Rohit, I made a pass on the KIP and it looks good to me too. Please feel free to start a voting thread. On Fri, Nov 13, 2020 at 7:36 AM John Roesler wrote: > Thanks Rohit! > > This looks good to me. As far as I’m concerned, you could start the voting > thread. > > -John > > On Wed, Nov 11

Re: [DISCUSS] KIP-680: TopologyTestDriver should not require a Properties argument

2020-11-13 Thread John Roesler
Thanks Rohit! This looks good to me. As far as I’m concerned, you could start the voting thread. -John On Wed, Nov 11, 2020, at 21:21, Rohit Deshpande wrote: > Hi John and Matthias, > I have updated the wiki with your suggestions. > https://cwiki.apache.org/confluence/display/KAFKA/KIP-680%3A+

Re: [DISCUSS] KIP-680: TopologyTestDriver should not require a Properties argument

2020-11-11 Thread Rohit Deshpande
Hi John and Matthias, I have updated the wiki with your suggestions. https://cwiki.apache.org/confluence/display/KAFKA/KIP-680%3A+TopologyTestDriver+should+not+require+a+Properties+argument Thanks, Rohit On Fri, Nov 6, 2020 at 3:23 PM Rohit Deshpande wrote: > Thanks John, I will go ahead and u

Re: [DISCUSS] KIP-680: TopologyTestDriver should not require a Properties argument

2020-11-06 Thread Rohit Deshpande
Thanks John, I will go ahead and update the KIP with a randomized application id requirement. On Fri, Nov 6, 2020 at 3:12 PM John Roesler wrote: > Hi Rohit, > > Ah, indeed, that was my mistake. I made a bad assumption about the code. > > Since we are already cleaning up, then I’d suggest only th

Re: [DISCUSS] KIP-680: TopologyTestDriver should not require a Properties argument

2020-11-06 Thread John Roesler
Hi Rohit, Ah, indeed, that was my mistake. I made a bad assumption about the code. Since we are already cleaning up, then I’d suggest only that we might generate a randomized application id so that concurrent tests won’t interfere with each other. But this is sounding like a minor implementatio

Re: [DISCUSS] KIP-680: TopologyTestDriver should not require a Properties argument

2020-11-06 Thread Rohit Deshpande
Hi John, Thank you for your review and the feedback. In existing method TTD.close(), stateDirectory.clean() method is getting called which is cleaning up task and g

Re: [DISCUSS] KIP-680: TopologyTestDriver should not require a Properties argument

2020-11-06 Thread John Roesler
Hello Rohit, Thanks for picking this up! I think your KIP looks good. While I was doing some cleanup of our tests before, one thing I encountered is that, while most tests don’t semantically need to specify any configs, many tests actually do set the state directory config. They set it specific

Re: [DISCUSS] KIP-680: TopologyTestDriver should not require a Properties argument

2020-11-03 Thread Rohit Deshpande
Hi Matthias, Thank you for the review and the suggestion. Considering at most 3 parameters to the constructor of TopologyTestDriver and topology being required parameter, we can definitely add a new constructor `TopologyTestDriver(Topology, Instant)` . Right now, I see one test where we can use thi

Re: [DISCUSS] KIP-680: TopologyTestDriver should not require a Properties argument

2020-11-03 Thread Matthias J. Sax
Thanks for the KIP Rohit. Wondering, if we should also add `TopologyTestDriver(Topology, Instant)`? Not totally sure, as having too many overload could also be annoying. -Matthias On 11/3/20 10:02 AM, Rohit Deshpande wrote: > Hello all, > I have created KIP-680: TopologyTestDriver should not re

[DISCUSS] KIP-680: TopologyTestDriver should not require a Properties argument

2020-11-03 Thread Rohit Deshpande
Hello all, I have created KIP-680: TopologyTestDriver should not require a Properties argument. https://cwiki.apache.org/confluence/display/KAFKA/KIP-680%3A+TopologyTestDriver+should+not+require+a+Properties+argument Jira for the KIP: https://issues.apache.org/jira/browse/KAFKA-10629 If we end up