Maybe one follow up question: Why do we pass `Serdes` into `createInputTopic` and createOutputTopic` -- seems `Serializer` (for input) and `Deserialized` (for output) should be sufficient?
-Matthias On 9/12/19 4:59 PM, Matthias J. Sax wrote: > Thanks for updating the KIP. > > I agree with John that we (ie, you :)) can start a vote. > > > -Matthias > > On 9/11/19 9:23 AM, John Roesler wrote: >> Thanks for the update, Jukka! >> >> I'd be in favor of the current proposal. Not sure how the others feel. >> If people generally feel positive, it might be time to start a vote. >> >> Thanks, >> -John >> >> On Sat, Sep 7, 2019 at 12:40 AM Jukka Karvanen >> <jukka.karva...@jukinimi.com> wrote: >>> >>> Hi, >>> >>> Sorry; I need to rollback right away the one method removal what I was >>> proposing. >>> >>> One constructor with Long restored to TestRecord, which is needed by >>> TestInputTopic. >>> >>> Regards, >>> Jukka >>> >>> la 7. syysk. 2019 klo 8.06 Jukka Karvanen (jukka.karva...@jukinimi.com) >>> kirjoitti: >>> >>>> Hi, >>>> >>>> Let's get back to this after summer break. >>>> Thanks Antony to offering help, it might be needed. >>>> >>>> I modified the KIP based on the feedback to be a mixture of variations 4 >>>> and 5. >>>> >>>> In TestInputTopic I removed deprecation from one variation with long >>>> timestamp and removed totally one version without key. >>>> The existing test code with it can be easily migrated to use remaining >>>> method adding null key. >>>> >>>> In TestRecord I removed constructors with Long timestamp from the public >>>> interface. You can migrate existing code >>>> with Long timestamp constructors to use constructors with ProducerRecord >>>> or ConsumerRecord. >>>> There is still Long timestamp(); method like in ProducerRecord / >>>> ConsumerRecord. >>>> >>>> Is this acceptable alternative? >>>> What else is needed to conclude the discussion phase and get to voting? >>>> >>>> Regards, >>>> Jukka >>>> >>>> to 5. syysk. 2019 klo 17.17 Antony Stubbs (ant...@confluent.io) kirjoitti: >>>> >>>>> Hi Jukka! I just came across your work - it looks great! I was taking a >>>>> stab at improving the existing API, but yours already looks great and just >>>>> about complete! Are you planning on continuing your work and submitting a >>>>> PR? If you want some help, I'd be happy to jump in. >>>>> >>>>> Regards, >>>>> Antony. >>>>> >>>>> On Thu, Aug 1, 2019 at 3:42 PM Bill Bejeck <bbej...@gmail.com> wrote: >>>>> >>>>>> Hi Jukka, >>>>>> >>>>>> I also think 3, 4, and 5 are all good options. >>>>>> >>>>>> My personal preference is 4, but I also wouldn't mind going with 5 if >>>>> that >>>>>> is what others want to do. >>>>>> >>>>>> Thanks, >>>>>> Bill >>>>>> >>>>>> On Tue, Jul 30, 2019 at 9:31 AM John Roesler <j...@confluent.io> wrote: >>>>>> >>>>>>> Hey Jukka, >>>>>>> >>>>>>> Sorry for the delay. >>>>>>> >>>>>>> For what it's worth, I think 3, 4, and 5 are all good options. I >>>>> guess my >>>>>>> own preference is 5. >>>>>>> >>>>>>> It seems like the migration pain is a one-time concern vs. having more >>>>>>> maintainable code for years thereafter. >>>>>>> >>>>>>> Thanks, >>>>>>> -John >>>>>>> >>>>>>> >>>>>>> >>>>>>> On Tue, Jul 2, 2019 at 4:03 AM Jukka Karvanen < >>>>>> jukka.karva...@jukinimi.com >>>>>>>> >>>>>>> wrote: >>>>>>> >>>>>>>> Hi Matthias, >>>>>>>> >>>>>>>> Generally I think using Instant and Duration make the test more >>>>>> readable >>>>>>>> and that's why I modified KIP based on your suggestion. >>>>>>>> Now a lot of code use time with long or Long and that make the >>>>> change >>>>>>> more >>>>>>>> complicated. >>>>>>>> >>>>>>>> What I tried to say about the migration is the lines without >>>>> timestamp >>>>>> or >>>>>>>> if long timestamp is supported can be migrated mainly with search & >>>>>>>> recplace: >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> testDriver.pipeInput(recordFactory.create(WordCountLambdaExample.inputTopic, >>>>>>>> nullKey, "Hello", 1L)); >>>>>>>> >>>>>>>> -> >>>>>>>> >>>>>>>> *inputTopic*.pipeInput(nullKey, "Hello", 1L); >>>>>>>> >>>>>>>> If long is not supported as timestamp, the same is not so easy: >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> testDriver.pipeInput(recordFactory.create(WordCountLambdaExample.inputTopic, >>>>>>>> nullKey, "Hello", 1L)); >>>>>>>> >>>>>>>> -> >>>>>>>> >>>>>>>> *inputTopic1*.pipeInput(nullKey, "Hello", Instant.ofEpochMilli(1L)); >>>>>>>> >>>>>>>> Also if you need to convert arbitrary long timestamps to proper time >>>>>>>> Instants, it require you need to understand the logic of the test. >>>>> So >>>>>>>> mechanical search & replace is not possible. >>>>>>>> >>>>>>>> >>>>>>>> I see there are several alternatives for long vs Instant / Duration: >>>>>>>> >>>>>>>> 1. All times as long/Long like in this version: >>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=119550011 >>>>>>>> >>>>>>>> (startTimestampMs, autoAdvanceMs as parameter of createInputTopic >>>>>>>> instead of configureTiming) >>>>>>>> >>>>>>>> 2. Auto timestamping configured with Instant and Duration, pipeInput >>>>>>>> and TestRecord with long: >>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=120722523 >>>>>>>> >>>>>>>> >>>>>>>> 3. (CURRENT) Auto timestamping configured with Instant and Duration, >>>>>>>> pipeInput and TestRecord with Instant, version with long deprecated: >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-470%3A+TopologyTestDriver+test+input+and+output+usability+improvements >>>>>>>> >>>>>>>> >>>>>>>> 4. Auto timestamping configured with Instant and Duration, pipeInput >>>>>>>> and TestRecord with Instant and long parallel (with long not >>>>>>>> deprecated): >>>>>>>> >>>>>>>> 5. Auto timestamping configured with Instant and Duration, pipeInput >>>>>>>> and TestRecord with Instant only >>>>>>>> >>>>>>>> I hope to get feedback. >>>>>>>> >>>>>>>> My own preference currently is alternative 3. or 4. >>>>>>>> >>>>>>>> >>>>>>>> If somebody want to test, there is a implementation of this version >>>>>>>> available in Github: >>>>>>>> >>>>>>>> https://github.com/jukkakarvanen/kafka-streams-test-topics >>>>>>>> >>>>>>>> which can be used directly from public Maven repository: >>>>>>>> >>>>>>>> <dependency> >>>>>>>> <groupId>com.github.jukkakarvanen</groupId> >>>>>>>> <artifactId>kafka-streams-test-topics</artifactId> >>>>>>>> <version>0.0.1-beta3</version> >>>>>>>> <scope>test</scope> >>>>>>>> </dependency> >>>>>>>> >>>>>>>> Also is this approach in KIP-470 preferred over KIP-456, so can we >>>>>> close >>>>>>>> it: >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-456%3A+Helper+classes+to+make+it+simpler+to+write+test+logic+with+TopologyTestDriver >>>>>>>> >>>>>>>> Jukka >>>>>>>> >>>>>>>> . >>>>>>>> >>>>>>>> >>>>>>>> pe 28. kesäk. 2019 klo 1.10 Matthias J. Sax (matth...@confluent.io) >>>>>>>> kirjoitti: >>>>>>>> >>>>>>>>> Thanks Jukka! >>>>>>>>> >>>>>>>>> The idea to use `Instant/Duration` was a proposal. If we think >>>>> it's >>>>>> not >>>>>>>>> a good one, we could still stay with `long`. Because >>>>> `ProducerRecord` >>>>>>>>> and `ConsumerRecord` are both based on `long,` it might make >>>>> sense to >>>>>>>>> keep `long`? >>>>>>>>> >>>>>>>>>> The result of converting millis to Instant directly generates >>>>>>>>>>> rather non readable test code and changing from long to Instant >>>>>>>>> correctly >>>>>>>>>>> require understand what is the case it is testing. >>>>>>>>> >>>>>>>>> This might be a good indicator the using `Instant/Duration` might >>>>> not >>>>>>> be >>>>>>>>> a good idea. >>>>>>>>> >>>>>>>>> Would be nice to get feedback from others. >>>>>>>>> >>>>>>>>> About adding new methods that we deprecate immediately: I don't >>>>> think >>>>>>> we >>>>>>>>> should do this. IMHO, there are two kind of users, one that >>>>>> immediately >>>>>>>>> rewrite their code to move off deprecated methods. Those won't use >>>>>> the >>>>>>>>> new+deprecated ones anyway. Other uses migrate their code slowly >>>>> and >>>>>>>>> would just not rewrite their code at all, and thus also not use >>>>> the >>>>>>>>> new+deprecated methods. >>>>>>>>> >>>>>>>>>> Checking my own tests I was able to migrate the most of my code >>>>>> with >>>>>>>>>> search&replace without further thinking about the logic to this >>>>> new >>>>>>>>>> approach. The result of converting millis to Instant directly >>>>>>> generates >>>>>>>>>> rather non readable test code and changing from long to Instant >>>>>>>> correctly >>>>>>>>>> require understand what is the case it is testing. >>>>>>>>> >>>>>>>>> Not sure if I can follow here. You first say, you could easily >>>>>> migrate >>>>>>>>> your code, but than you say it was not easily possible? Can you >>>>>> clarify >>>>>>>>> your experience upgrading your test code? >>>>>>>>> >>>>>>>>> >>>>>>>>> -Matthias >>>>>>>>> >>>>>>>>> >>>>>>>>> On 6/27/19 12:21 AM, Jukka Karvanen wrote: >>>>>>>>>> Hi, >>>>>>>>>> >>>>>>>>>>>> (4) Should we switch from `long` for timestamps to `Instant` >>>>> and >>>>>>>>>> `Duration` ? >>>>>>>>>>> This version startTimestamp is Instant and autoAdvance >>>>> Duration in >>>>>>>>>> Initialization and with manual configured collection pipe >>>>> methods. >>>>>>>>>>> Now timestamp of TestRecord is still Long and similarly single >>>>>>> record >>>>>>>>>> pipeInput still has long as parameter. >>>>>>>>>>> Should these also converted to to Instant type? >>>>>>>>>>> Should there be both long and Instant parallel? >>>>>>>>>>> I expect there are existing test codebase which would be >>>>> easier to >>>>>>>>> migrate >>>>>>>>>> if long could be still used. >>>>>>>>>> Now added Instant version to TestRecord and pipeInput method. >>>>>>>>>> >>>>>>>>>> Checking my own tests I was able to migrate the most of my code >>>>>> with >>>>>>>>>> search&replace without further thinking about the logic to this >>>>> new >>>>>>>>>> approach. The result of converting millis to Instant directly >>>>>>> generates >>>>>>>>>> rather non readable test code and changing from long to Instant >>>>>>>> correctly >>>>>>>>>> require understand what is the case it is testing. >>>>>>>>>> >>>>>>>>>> That is why version with long left still as deprecated for >>>>> easier >>>>>>>>> migration >>>>>>>>>> for existing test. >>>>>>>>>> Also TopologyTestDriver constructor and advanceWallClockTime >>>>>> method >>>>>>>>>> modified with same approach. >>>>>>>>>> >>>>>>>>>> Jukka >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> ma 24. kesäk. 2019 klo 16.47 Bill Bejeck (bbej...@gmail.com) >>>>>>>> kirjoitti: >>>>>>>>>> >>>>>>>>>>> Hi Jukka >>>>>>>>>>> >>>>>>>>>>>> These topic objects are only interfacing TopologyTestDriver, >>>>> not >>>>>>>>>>> affecting >>>>>>>>>>>> the internal functionality of it. In my plan the internal data >>>>>>>>> structures >>>>>>>>>>>> are using those Producer/ConsumerRecords as earlier. That way >>>>> I >>>>>>> don't >>>>>>>>> see >>>>>>>>>>>> how those could be affected. >>>>>>>>>>> >>>>>>>>>>> I mistakenly thought the KIP was proposing to completely remove >>>>>>>>>>> Producer/ConsumerRecords in favor of TestRecord. But taking >>>>>> another >>>>>>>>> quick >>>>>>>>>>> look I can see the plan for using the TestRecord objects. >>>>> Thanks >>>>>>> for >>>>>>>>> the >>>>>>>>>>> clarification. >>>>>>>>>>> >>>>>>>>>>> -Bill >>>>>>>>>>> >>>>>>>>>>> On Sat, Jun 22, 2019 at 2:26 AM Jukka Karvanen < >>>>>>>>>>> jukka.karva...@jukinimi.com> >>>>>>>>>>> wrote: >>>>>>>>>>> >>>>>>>>>>>> Hi All, >>>>>>>>>>>> Hi Matthias, >>>>>>>>>>>> >>>>>>>>>>>>> (1) It's a little confusing that you list all method >>>>> (existing, >>>>>>>>> proposed >>>>>>>>>>>>> to deprecate, and new one) of `TopologyTestDriver` in the >>>>> KIP. >>>>>>> Maybe >>>>>>>>>>>>> only list the ones you propose to deprecate and the new ones >>>>> you >>>>>>>> want >>>>>>>>> to >>>>>>>>>>>>> add? >>>>>>>>>>>> >>>>>>>>>>>> Ok. Unmodified methods removed. >>>>>>>>>>>> >>>>>>>>>>>>> (2) `TopologyTestDriver#createInputTopic`: might it be worth >>>>> to >>>>>>> add >>>>>>>>>>>>> overload to initialize the timetamp and auto-advance feature >>>>>>>> directly? >>>>>>>>>>>>> Otherwise, uses always need to call `configureTiming` as an >>>>>> extra >>>>>>>>> call? >>>>>>>>>>>> >>>>>>>>>>>> Added with Instant and Duration parameters. >>>>>>>>>>>> >>>>>>>>>>>>> (3) `TestInputTopic#configureTiming()`: maybe rename to >>>>>>>>>>>> `reconfigureTiming()` ? >>>>>>>>>>>> >>>>>>>>>>>> I removed this method when we have now initialization in >>>>>>> constructor. >>>>>>>>>>>> You can recreate TestInputTopic if needing to reconfigure >>>>> timing. >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>>> (4) Should we switch from `long` for timestamps to `Instant` >>>>> and >>>>>>>>>>>> `Duration` ? >>>>>>>>>>>> This version startTimestamp is Instant and autoAdvance >>>>> Duration >>>>>> in >>>>>>>>>>>> Initialization and with manual configured collection pipe >>>>>> methods. >>>>>>>>>>>> Now timestamp of TestRecord is still Long and similarly single >>>>>>> record >>>>>>>>>>>> pipeInput still has long as parameter. >>>>>>>>>>>> Should these also converted to to Instant type? >>>>>>>>>>>> Should there be both long and Instant parallel? >>>>>>>>>>>> I expect there are existing test codebase which would be >>>>> easier >>>>>> to >>>>>>>>>>> migrate >>>>>>>>>>>> if long could be still used. >>>>>>>>>>>> >>>>>>>>>>>> Also should advanceWallClockTime in TopologyTestDriver >>>>>> changed(or >>>>>>>>> added >>>>>>>>>>>> alternative) for Duration parameter. >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>>> (5) Why do we have redundant getters? Or set with `getX()` >>>>> and >>>>>> one >>>>>>>>>>>> set without `get`-prefix? >>>>>>>>>>>> >>>>>>>>>>>> The methods without get-prefix are for compatibility with >>>>>>>>>>> ProducerRecord / >>>>>>>>>>>> ConsumerRecord and I expect would make migration to TestRecord >>>>>>>> easier. >>>>>>>>>>>> Standard getters in TestRecord enable writing test ignoring >>>>>>> selected >>>>>>>>>>> fields >>>>>>>>>>>> with hamcrest like this: >>>>>>>>>>>> >>>>>>>>>>>> assertThat(outputTopic.readRecord(), allOf( >>>>>>>>>>>> hasProperty("key", equalTo(1L)), >>>>>>>>>>>> hasProperty("value", equalTo("Hello")), >>>>>>>>>>>> hasProperty("headers", equalTo(headers)))); >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> That's why I have currently both methods. >>>>>>>>>>>> >>>>>>>>>>>> Jukka >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> pe 21. kesäk. 2019 klo 22.20 Matthias J. Sax ( >>>>>>> matth...@confluent.io) >>>>>>>>>>>> kirjoitti: >>>>>>>>>>>> >>>>>>>>>>>>> Thanks for the KIP. The idea to add InputTopic and >>>>> OutputTopic >>>>>>>>>>>>> abstractions is really neat! >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> Couple of minor comment: >>>>>>>>>>>>> >>>>>>>>>>>>> (1) It's a little confusing that you list all method >>>>> (existing, >>>>>>>>>>> proposed >>>>>>>>>>>>> to deprecate, and new one) of `TopologyTestDriver` in the >>>>> KIP. >>>>>>> Maybe >>>>>>>>>>>>> only list the ones you propose to deprecate and the new ones >>>>> you >>>>>>>> want >>>>>>>>>>> to >>>>>>>>>>>>> add? >>>>>>>>>>>>> >>>>>>>>>>>>> (Or mark all existing methods clearly -- atm, I need to got >>>>> back >>>>>>> to >>>>>>>>> the >>>>>>>>>>>>> code to read the KIP and to extract what changes are >>>>> proposed). >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> (2) `TopologyTestDriver#createInputTopic`: might it be worth >>>>> to >>>>>>> add >>>>>>>>>>>>> overload to initialize the timetamp and auto-advance feature >>>>>>>> directly? >>>>>>>>>>>>> Otherwise, uses always need to call `configureTiming` as an >>>>>> extra >>>>>>>>> call? >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> (3) `TestInputTopic#configureTiming()`: maybe rename to >>>>>>>>>>>>> `reconfigureTiming()` ? >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> (4) Should we switch from `long` for timestamps to `Instant` >>>>> and >>>>>>>>>>>>> `Duration` ? >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> (5) Why do we have redundant getters? Or set with `getX()` >>>>> and >>>>>> one >>>>>>>> set >>>>>>>>>>>>> without `get`-prefix? >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> -Matthias >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> On 6/21/19 10:57 AM, Bill Bejeck wrote: >>>>>>>>>>>>>> Jukka, >>>>>>>>>>>>>> >>>>>>>>>>>>>> Thanks for the KIP. I like the changes overall. >>>>>>>>>>>>>> One thing I wanted to confirm, and this may be me being >>>>>> paranoid, >>>>>>>> but >>>>>>>>>>>>> will >>>>>>>>>>>>>> the changes for input/output topic affect how the >>>>>>>> TopologyTestDriver >>>>>>>>>>>>> works >>>>>>>>>>>>>> with internal topics when there are sub-topologies created? >>>>>>>>>>>>>> >>>>>>>>>>>>>> On Fri, Jun 21, 2019 at 12:05 PM Guozhang Wang < >>>>>>> wangg...@gmail.com >>>>>>>>> >>>>>>>>>>>>> wrote: >>>>>>>>>>>>>> >>>>>>>>>>>>>>> 1) Got it, could you list this class along with all its >>>>>>> functions >>>>>>>> in >>>>>>>>>>>> the >>>>>>>>>>>>>>> proposed public APIs as well? >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> 2) Ack, thanks! >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> On Thu, Jun 20, 2019 at 11:27 PM Jukka Karvanen < >>>>>>>>>>>>>>> jukka.karva...@jukinimi.com> >>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> Hi Guozhang, >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> 1) This TestRecord is new class in my proposal. So it is a >>>>>>>>>>> simplified >>>>>>>>>>>>>>>> version of ProducerRecord and ConsumerRecord containing >>>>> only >>>>>>> the >>>>>>>>>>>> fields >>>>>>>>>>>>>>>> needed to test record content. >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> 2) >>>>>>>>>>>>>>>> public final <K, V> TestInputTopic<K, V> >>>>>> createInputTopic(final >>>>>>>>>>>> String >>>>>>>>>>>>>>>> topicName, final Serde<K> keySerde, final Serde<V> >>>>>> valueSerde); >>>>>>>>>>>>>>>> public final <K, V> TestOutputTopic<K, V> >>>>>>> createOutputTopic(final >>>>>>>>>>>>> String >>>>>>>>>>>>>>>> topicName, final Serde<K> keySerde, final Serde<V> >>>>>> valueSerde); >>>>>>>>>>>>>>>> The purpose is to create separate object for each input >>>>> and >>>>>>>> output >>>>>>>>>>>>> topic >>>>>>>>>>>>>>>> you are using. The topic name is given to >>>>>>> createInput/OutputTopic >>>>>>>>>>>> when >>>>>>>>>>>>>>>> initialize topic object. >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> For example: >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> final TestInputTopic<Long, String> inputTopic1 = >>>>>>>>>>>>>>>> testDriver.createInputTopic( INPUT_TOPIC, longSerde, >>>>>>>> stringSerde); >>>>>>>>>>>>>>>> final TestInputTopic<Long, String> inputTopic2 = >>>>>>>>>>>>>>>> testDriver.createInputTopic( INPUT_TOPIC_MAP, longSerde, >>>>>>>>>>>> stringSerde); >>>>>>>>>>>>>>>> final TestOutputTopic<Long, String> outputTopic1 = >>>>>>>>>>>>>>>> testDriver.createOutputTopic(OUTPUT_TOPIC, longSerde, >>>>>>>> stringSerde); >>>>>>>>>>>>>>>> final TestOutputTopic<String, Long> outputTopic2 = >>>>>>>>>>>>>>>> testDriver.createOutputTopic(OUTPUT_TOPIC_MAP, >>>>> stringSerde, >>>>>>>>>>>>>>>> longSerde); >>>>>>>>>>>>>>>> inputTopic1.pipeInput(1L, "Hello"); >>>>>>>>>>>>>>>> assertThat(outputTopic1.readKeyValue(), equalTo(new >>>>>>>> KeyValue<>(1L, >>>>>>>>>>>>>>>> "Hello"))); >>>>>>>>>>>>>>>> assertThat(outputTopic2.readKeyValue(), equalTo(new >>>>>>>>>>>> KeyValue<>("Hello", >>>>>>>>>>>>>>>> 1L))); >>>>>>>>>>>>>>>> inputTopic2.pipeInput(1L, "Hello"); >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> Jukka >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> to 20. kesäk. 2019 klo 23.52 Guozhang Wang ( >>>>>> wangg...@gmail.com >>>>>>> ) >>>>>>>>>>>>>>> kirjoitti: >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> Hello Jukka, >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> Thanks for writing the KIP, I have a couple of quick >>>>>>> questions: >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> 1) Is "TestRecord" an existing class that you propose to >>>>>>>>>>> piggy-back >>>>>>>>>>>>> on? >>>>>>>>>>>>>>>>> Right now we have a scala TestRecord case class but I >>>>> doubt >>>>>>> that >>>>>>>>>>> was >>>>>>>>>>>>>>> your >>>>>>>>>>>>>>>>> proposal, or are you proposing to add a new Java class? >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> 2) Would the new API only allow a single input / output >>>>>> topic >>>>>>>> with >>>>>>>>>>>>>>>>> `createInput/OutputTopic`? If not, when we call pipeInput >>>>>> how >>>>>>> to >>>>>>>>>>>>>>>> determine >>>>>>>>>>>>>>>>> which topic this record should be pipe to? >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> Guozhang >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> On Mon, Jun 17, 2019 at 1:34 PM John Roesler < >>>>>>> j...@confluent.io >>>>>>>>> >>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> Woah, I wasn't aware of that Hamcrest test style. >>>>> Awesome! >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> Thanks for the updates. I look forward to hearing what >>>>>> others >>>>>>>>>>>> think. >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> -John >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> On Mon, Jun 17, 2019 at 4:12 AM Jukka Karvanen >>>>>>>>>>>>>>>>>> <jukka.karva...@jukinimi.com> wrote: >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> Wiki page updated: >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-470%3A+TopologyTestDriver+test+input+and+output+usability+improvements >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> ClientRecord removed and replaced with TestRecord in >>>>>> method >>>>>>>>>>> calls. >>>>>>>>>>>>>>>>>>> TestRecordFactory removed (time tracking functionality >>>>> to >>>>>> be >>>>>>>>>>>>>>> included >>>>>>>>>>>>>>>>> to >>>>>>>>>>>>>>>>>>> TestInputTopic) >>>>>>>>>>>>>>>>>>> OutputVerifier deprecated >>>>>>>>>>>>>>>>>>> TestRecord topic removed and getters added >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> Getters in TestRecord enable writing test ignoring >>>>>> selected >>>>>>>>>>> fields >>>>>>>>>>>>>>>> with >>>>>>>>>>>>>>>>>>> hamcrest like this: >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> assertThat(outputTopic.readRecord(), allOf( >>>>>>>>>>>>>>>>>>> hasProperty("key", equalTo(1L)), >>>>>>>>>>>>>>>>>>> hasProperty("value", equalTo("Hello")), >>>>>>>>>>>>>>>>>>> hasProperty("headers", equalTo(headers)))); >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> Jukka >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> la 15. kesäk. 2019 klo 1.10 John Roesler ( >>>>>> j...@confluent.io >>>>>>> ) >>>>>>>>>>>>>>>>> kirjoitti: >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> Sounds good. Thanks as always for considering my >>>>>> feedback! >>>>>>>>>>>>>>>>>>>> -John >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> On Fri, Jun 14, 2019 at 12:12 PM Jukka Karvanen >>>>>>>>>>>>>>>>>>>> <jukka.karva...@jukinimi.com> wrote: >>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>> Ok, I will modify KIP Public Interface in a wiki >>>>> based >>>>>> on >>>>>>>> the >>>>>>>>>>>>>>>>>> feedback. >>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>> TestRecordFactory / ConsumerRecordFactory was used by >>>>>>>>>>>>>>>>> TestInputTopic >>>>>>>>>>>>>>>>>> with >>>>>>>>>>>>>>>>>>>>> the version I had with KIP456, but maybe I can merge >>>>>> That >>>>>>>>>>>>>>>>>> functionality >>>>>>>>>>>>>>>>>>>> to >>>>>>>>>>>>>>>>>>>>> InputTopic or TestRecordFactory can kept non >>>>> public >>>>>>> maybe >>>>>>>>>>>>>>>> moving >>>>>>>>>>>>>>>>>> it to >>>>>>>>>>>>>>>>>>>>> internals package. >>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>> I will make the proposal with a slim down interface. >>>>>>>>>>>>>>>>>>>>> I don't want to go to so slim as you proposed with >>>>> only >>>>>>>>>>>>>>>> TestRecord >>>>>>>>>>>>>>>>> or >>>>>>>>>>>>>>>>>>>>> List<TestRecord>, because you then still end up doing >>>>>>> helper >>>>>>>>>>>>>>>>> methods >>>>>>>>>>>>>>>>>> to >>>>>>>>>>>>>>>>>>>>> construct List of TestRecord. >>>>>>>>>>>>>>>>>>>>> The list of values is easier to write and clearer to >>>>>> read >>>>>>>> than >>>>>>>>>>>>>>> if >>>>>>>>>>>>>>>>> you >>>>>>>>>>>>>>>>>>>> need >>>>>>>>>>>>>>>>>>>>> to contruct list of TestRecords. >>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>> For example: >>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>> final List<String> inputValues = Arrays.asList( >>>>>>>>>>>>>>>>>>>>> "Apache Kafka Streams Example", >>>>>>>>>>>>>>>>>>>>> "Using Kafka Streams Test Utils", >>>>>>>>>>>>>>>>>>>>> "Reading and Writing Kafka Topic" >>>>>>>>>>>>>>>>>>>>> ); >>>>>>>>>>>>>>>>>>>>> inputTopic.pipeValueList(inputValues); >>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>> Let's check after the next iteration is it still >>>>> worth >>>>>>>>>>> reducing >>>>>>>>>>>>>>>> the >>>>>>>>>>>>>>>>>>>> methods. >>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>> Jukka >>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>> pe 14. kesäk. 2019 klo 18.27 John Roesler ( >>>>>>>> j...@confluent.io) >>>>>>>>>>>>>>>>>> kirjoitti: >>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>> Thanks, Jukka, >>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>> Ok, I buy this reasoning. >>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>> Just to echo what I think I read, you would just >>>>> drop >>>>>>>>>>>>>>>>> ClientRecord >>>>>>>>>>>>>>>>>>>>>> from the proposal, and TestRecord would stand on its >>>>>> own, >>>>>>>>>>>>>>> with >>>>>>>>>>>>>>>>> the >>>>>>>>>>>>>>>>>>>>>> same methods and properties you proposed, and the >>>>>> "input >>>>>>>>>>>>>>> topic" >>>>>>>>>>>>>>>>>> would >>>>>>>>>>>>>>>>>>>>>> accept TestRecord, and the "output topic" would >>>>> produce >>>>>>>>>>>>>>>>> TestRecord? >>>>>>>>>>>>>>>>>>>>>> Further, the "input and output topic" classes would >>>>>>>>>>>>>>> internally >>>>>>>>>>>>>>>>>> handle >>>>>>>>>>>>>>>>>>>>>> the conversion to and from ConsumerRecord and >>>>>>>> ProducerRecord >>>>>>>>>>>>>>> to >>>>>>>>>>>>>>>>>> pass >>>>>>>>>>>>>>>>>>>>>> to and from the TopologyTestDriver? >>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>> This seems good to me. >>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>> Since the object coming out of the "output topic" is >>>>>> much >>>>>>>>>>>>>>> more >>>>>>>>>>>>>>>>>>>>>> ergonomic, I suspect we won't need the >>>>> OutputVerifier >>>>>> at >>>>>>>> all. >>>>>>>>>>>>>>>> It >>>>>>>>>>>>>>>>>> was >>>>>>>>>>>>>>>>>>>>>> mostly needed because of all the extra junk in >>>>>>>> ProducerRecord >>>>>>>>>>>>>>>> you >>>>>>>>>>>>>>>>>> need >>>>>>>>>>>>>>>>>>>>>> to ignore. It seems better to just deprecate it. If >>>>> in >>>>>>> the >>>>>>>>>>>>>>>> future >>>>>>>>>>>>>>>>>> it >>>>>>>>>>>>>>>>>>>>>> turns out there is some actual use case for a >>>>> verifier, >>>>>>> we >>>>>>>>>>>>>>> can >>>>>>>>>>>>>>>>>> have a >>>>>>>>>>>>>>>>>>>>>> very small KIP to add one. But reading your >>>>> response, I >>>>>>>>>>>>>>> suspect >>>>>>>>>>>>>>>>>> that >>>>>>>>>>>>>>>>>>>>>> existing test verification libraries would be >>>>>> sufficient >>>>>>> on >>>>>>>>>>>>>>>> their >>>>>>>>>>>>>>>>>> own. >>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>> Similarly, it seems like we can shrink the total >>>>>>> interface >>>>>>>> by >>>>>>>>>>>>>>>>>> removing >>>>>>>>>>>>>>>>>>>>>> the TestRecordFactory from the proposal. If >>>>> TestRecord >>>>>>>>>>>>>>> already >>>>>>>>>>>>>>>>>> offers >>>>>>>>>>>>>>>>>>>>>> all the constructors you'd want, then the only >>>>> benefit >>>>>> of >>>>>>>> the >>>>>>>>>>>>>>>>>> factory >>>>>>>>>>>>>>>>>>>>>> is to auto-increment the timestamps, but then again, >>>>>> the >>>>>>>>>>>>>>> "input >>>>>>>>>>>>>>>>>> topic" >>>>>>>>>>>>>>>>>>>>>> can already do that (e.g., it can do it if the >>>>> record >>>>>>>>>>>>>>> timestamp >>>>>>>>>>>>>>>>> is >>>>>>>>>>>>>>>>>> not >>>>>>>>>>>>>>>>>>>>>> set). >>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>> Likewise, if the TestRecords are easy to create, >>>>> then >>>>>> we >>>>>>>>>>>>>>> don't >>>>>>>>>>>>>>>>> need >>>>>>>>>>>>>>>>>>>>>> all the redundant methods in "input topic" to pipe >>>>>>> values, >>>>>>>> or >>>>>>>>>>>>>>>>>>>>>> key/values, or key/value/timestamp, etc. We can do >>>>> with >>>>>>>> just >>>>>>>>>>>>>>>> two >>>>>>>>>>>>>>>>>>>>>> methods, one for a single TestRecord and one for a >>>>>>>> collection >>>>>>>>>>>>>>>> of >>>>>>>>>>>>>>>>>> them. >>>>>>>>>>>>>>>>>>>>>> This reduces API ambiguity and also dramatically >>>>>>> decreases >>>>>>>>>>>>>>> the >>>>>>>>>>>>>>>>>> surface >>>>>>>>>>>>>>>>>>>>>> area of the interface, which ultimately makes it >>>>> much >>>>>>>> easier >>>>>>>>>>>>>>> to >>>>>>>>>>>>>>>>>> use. >>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>> It's best to start with the smallest interface that >>>>>> will >>>>>>> do >>>>>>>>>>>>>>> the >>>>>>>>>>>>>>>>> job >>>>>>>>>>>>>>>>>>>>>> and expand it upon request, rather than throwing in >>>>>>>>>>>>>>> everything >>>>>>>>>>>>>>>>> you >>>>>>>>>>>>>>>>>> can >>>>>>>>>>>>>>>>>>>>>> think of up front. The extra stuff would be >>>>> confusing >>>>>> to >>>>>>>>>>>>>>> people >>>>>>>>>>>>>>>>>> facing >>>>>>>>>>>>>>>>>>>>>> two practically identical paths to accomplish the >>>>> same >>>>>>>> goal, >>>>>>>>>>>>>>>> and >>>>>>>>>>>>>>>>>> it's >>>>>>>>>>>>>>>>>>>>>> very difficult to slim the interface down later, >>>>>> because >>>>>>> we >>>>>>>>>>>>>>>> don't >>>>>>>>>>>>>>>>>>>>>> really know which parts are more popular (i.e., no >>>>> one >>>>>>>>>>>>>>> submits >>>>>>>>>>>>>>>>>>>>>> "feature requests" to _remove_ stuff they don't >>>>> need, >>>>>>> only >>>>>>>> to >>>>>>>>>>>>>>>>> _add_ >>>>>>>>>>>>>>>>>>>>>> stuff that they need. >>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>> But overall, I really like the structure of this >>>>>> design. >>>>>>>> I'm >>>>>>>>>>>>>>>>> super >>>>>>>>>>>>>>>>>>>>>> excited about this KIP. >>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>> Thanks, >>>>>>>>>>>>>>>>>>>>>> -John >>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>> On Fri, Jun 14, 2019 at 2:55 AM Jukka Karvanen >>>>>>>>>>>>>>>>>>>>>> <jukka.karva...@jukinimi.com> wrote: >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>> Hi, >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>> I am not a fan of swapping only ProducerRecord and >>>>>>>>>>>>>>>>>> ConsumerRecord. >>>>>>>>>>>>>>>>>>>>>>> As a test writer point of view I do not want to >>>>> care >>>>>>> about >>>>>>>>>>>>>>>> the >>>>>>>>>>>>>>>>>>>> difference >>>>>>>>>>>>>>>>>>>>>>> of those and >>>>>>>>>>>>>>>>>>>>>>> that way I like to have object type which can be >>>>> used >>>>>> to >>>>>>>>>>>>>>> pipe >>>>>>>>>>>>>>>>>>>> records in >>>>>>>>>>>>>>>>>>>>>>> and compare outputs. >>>>>>>>>>>>>>>>>>>>>>> That way avoid unnecessary conversions between >>>>>>>>>>>>>>> ProducerRecord >>>>>>>>>>>>>>>>> and >>>>>>>>>>>>>>>>>>>>>>> ConsumerRecord. >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>> My initial assumption was that ProducerRecord and >>>>>>>>>>>>>>>>>>>> ConsumerRecord.could >>>>>>>>>>>>>>>>>>>>>>> implement the same ClientRecord >>>>>>>>>>>>>>>>>>>>>>> and that way test writer could have used either of >>>>>>> those, >>>>>>>>>>>>>>> but >>>>>>>>>>>>>>>>>> seems >>>>>>>>>>>>>>>>>>>> that >>>>>>>>>>>>>>>>>>>>>>> return type of timestamp method long vs Long is not >>>>>>>>>>>>>>>> compatible. >>>>>>>>>>>>>>>>>>>>>>> Now the main advantage of ClientRecord is no need >>>>> to >>>>>>>>>>>>>>>> duplicate >>>>>>>>>>>>>>>>>>>>>>> OutputVerifier when it is modified from >>>>> ProducerRecord >>>>>>> to >>>>>>>>>>>>>>>>>>>> ClientRecord. >>>>>>>>>>>>>>>>>>>>>>> Generally is there need for OutputVerifier. Can we >>>>>>>>>>>>>>> deprecate >>>>>>>>>>>>>>>>> the >>>>>>>>>>>>>>>>>>>> existing >>>>>>>>>>>>>>>>>>>>>>> and use standard assertion libraries for new test. >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>> If you use hamcrest, assert-j or any other >>>>> assertion >>>>>>>>>>>>>>> library >>>>>>>>>>>>>>>>>> for the >>>>>>>>>>>>>>>>>>>>>> rest >>>>>>>>>>>>>>>>>>>>>>> of the test, why not use it with these also. >>>>>>>>>>>>>>>>>>>>>>> When we have these methods to access only needed >>>>>> fields >>>>>>> it >>>>>>>>>>>>>>> is >>>>>>>>>>>>>>>>>> easier >>>>>>>>>>>>>>>>>>>> to >>>>>>>>>>>>>>>>>>>>>>> write test like this: >>>>>>>>>>>>>>>>>>>>>>> >>>>>> assertThat(outputTopic.readValue()).isEqualTo("Hello"); >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>> or >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >>>>>> assertThat(outputTopic.readRecord()).isEqualTo(expectedRecord); >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>> Only value for new OutputVerifier is when needing >>>>> to >>>>>>>> ignore >>>>>>>>>>>>>>>>> some >>>>>>>>>>>>>>>>>>>> fields >>>>>>>>>>>>>>>>>>>>>>> ClientRecord actual = outputTopic.readRecord(); >>>>>>>>>>>>>>>>>>>>>>> assertThat(actual.value()).isEqualTo("Hello"); >>>>>>>>>>>>>>>>>>>>>>> assertThat(actual.key()).isEqualTo(expectedKey); >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>> assertThat(actual.timestamp()).isEqualTo(expectedTimestamp); >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>> So if want to leave client package untouched, I >>>>> would >>>>>>>>>>>>>>> modify >>>>>>>>>>>>>>>>> the >>>>>>>>>>>>>>>>>>>> methods >>>>>>>>>>>>>>>>>>>>>>> with ClientRecord now in InputTopic and >>>>> OutputTopic to >>>>>>>> pass >>>>>>>>>>>>>>>> in >>>>>>>>>>>>>>>>>> and >>>>>>>>>>>>>>>>>>>> out >>>>>>>>>>>>>>>>>>>>>> this >>>>>>>>>>>>>>>>>>>>>>> TestRecord instead. >>>>>>>>>>>>>>>>>>>>>>> In that case there would be possibility to add >>>>> methods >>>>>>> to >>>>>>>>>>>>>>>>>> TestRecord >>>>>>>>>>>>>>>>>>>> to >>>>>>>>>>>>>>>>>>>>>>> help ignore some fields in assertions like: >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> assertThat(outputTopic.readRecord().getValueTimestamp()).isEqualTo(expectedRecord.get >>>>>>>>>>>>>>>>>>>>>>> ValueTimestamp()); >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>> How about this alternative? >>>>>>>>>>>>>>>>>>>>>>> If this way sounds better I will modify KIP page in >>>>>>> wiki. >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>> Jukka >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>> to 13. kesäk. 2019 klo 18.30 John Roesler ( >>>>>>>>>>>>>>> j...@confluent.io >>>>>>>>>>>>>>>> ) >>>>>>>>>>>>>>>>>>>> kirjoitti: >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> Hey, all, maybe we can jump-start this discussion. >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> I think this approach would be very ergonomic for >>>>>>>>>>>>>>> testing, >>>>>>>>>>>>>>>>> and >>>>>>>>>>>>>>>>>>>> would >>>>>>>>>>>>>>>>>>>>>>>> help reduce boilerplate in tests. >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> The think I like most about it is that it mirrors >>>>> the >>>>>>>>>>>>>>>> mental >>>>>>>>>>>>>>>>>> model >>>>>>>>>>>>>>>>>>>>>>>> that people already have from Kafka Streams, in >>>>> which >>>>>>> you >>>>>>>>>>>>>>>>>> write to >>>>>>>>>>>>>>>>>>>> an >>>>>>>>>>>>>>>>>>>>>>>> "input topic" and then get your results from an >>>>>> "output >>>>>>>>>>>>>>>>>> topic". As >>>>>>>>>>>>>>>>>>>> a >>>>>>>>>>>>>>>>>>>>>>>> side benefit, the input and output topics in the >>>>>>> proposal >>>>>>>>>>>>>>>>> also >>>>>>>>>>>>>>>>>>>> close >>>>>>>>>>>>>>>>>>>>>>>> over the serdes, which makes it much less >>>>> boilerplate >>>>>>> for >>>>>>>>>>>>>>>>> test >>>>>>>>>>>>>>>>>>>> code. >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> If I can offer one suggestion for change, I'm not >>>>>> sure >>>>>>>>>>>>>>> I'm >>>>>>>>>>>>>>>>>> totally >>>>>>>>>>>>>>>>>>>>>>>> sold on the need for a new abstraction >>>>> "ClientRecord" >>>>>>>>>>>>>>> with >>>>>>>>>>>>>>>> an >>>>>>>>>>>>>>>>>>>>>>>> implementation for tests "TestRecord". It seems >>>>> like >>>>>>> this >>>>>>>>>>>>>>>>>>>>>>>> unnecessarily complicates the main (non-testing) >>>>> data >>>>>>>>>>>>>>>> model. >>>>>>>>>>>>>>>>> It >>>>>>>>>>>>>>>>>>>> seems >>>>>>>>>>>>>>>>>>>>>>>> like it would be sufficient, and just as >>>>> ergonomic, >>>>>> to >>>>>>>>>>>>>>> have >>>>>>>>>>>>>>>>> the >>>>>>>>>>>>>>>>>>>> input >>>>>>>>>>>>>>>>>>>>>>>> topic accept ProducerRecords and the output topic >>>>>>> return >>>>>>>>>>>>>>>>>>>>>>>> ConsumerRecords. I'm open to discussion on this >>>>>> point, >>>>>>>>>>>>>>>>> though. >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> Thanks for the proposal, Jukka! >>>>>>>>>>>>>>>>>>>>>>>> -John >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> On Mon, May 20, 2019 at 7:59 AM Jukka Karvanen >>>>>>>>>>>>>>>>>>>>>>>> <jukka.karva...@jukinimi.com> wrote: >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> Hi All, >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> I would like to start the discussion on KIP-470: >>>>>>>>>>>>>>>>>>>> TopologyTestDriver >>>>>>>>>>>>>>>>>>>>>> test >>>>>>>>>>>>>>>>>>>>>>>>> input and output usability improvements: >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-470%3A+TopologyTestDriver+test+input+and+output+usability+improvements >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> This KIP is inspired by the Discussion in >>>>> KIP-456: >>>>>>>>>>>>>>> Helper >>>>>>>>>>>>>>>>>>>> classes to >>>>>>>>>>>>>>>>>>>>>> make >>>>>>>>>>>>>>>>>>>>>>>>> it simpler to write test logic with >>>>>>> TopologyTestDriver: >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-456 >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> %3A+Helper+classes+to+make+it+simpler+to+write+test+logic+with+TopologyTestDriver >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> The proposal in KIP-456 >>>>>>>>>>>>>>>>>>>>>>>>> < >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-456%3A+Helper+classes+to+make+it+simpler+to+write+test+logic+with+TopologyTestDriver >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> was >>>>>>>>>>>>>>>>>>>>>>>>> to add alternate way to input and output topic, >>>>> but >>>>>>>>>>>>>>> this >>>>>>>>>>>>>>>>> KIP >>>>>>>>>>>>>>>>>>>> enhance >>>>>>>>>>>>>>>>>>>>>>>> those >>>>>>>>>>>>>>>>>>>>>>>>> classes and deprecate old functionality to make >>>>>> clear >>>>>>>>>>>>>>>>>> interface >>>>>>>>>>>>>>>>>>>> for >>>>>>>>>>>>>>>>>>>>>> test >>>>>>>>>>>>>>>>>>>>>>>>> writer to use. >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> In current KIP-470 proposal, topic objects are >>>>>> created >>>>>>>>>>>>>>>> with >>>>>>>>>>>>>>>>>>>>>> topicName and >>>>>>>>>>>>>>>>>>>>>>>>> related serders. >>>>>>>>>>>>>>>>>>>>>>>>> public final <K, V> TestInputTopic<K, V> >>>>>>>>>>>>>>>>>>>> createInputTopic(final >>>>>>>>>>>>>>>>>>>>>>>> String >>>>>>>>>>>>>>>>>>>>>>>>> topicName, final Serde<K> keySerde, final >>>>> Serde<V> >>>>>>>>>>>>>>>>>> valueSerde); >>>>>>>>>>>>>>>>>>>>>>>>> public final <K, V> TestOutputTopic<K, V> >>>>>>>>>>>>>>>>>>>> createOutputTopic(final >>>>>>>>>>>>>>>>>>>>>>>> String >>>>>>>>>>>>>>>>>>>>>>>>> topicName, final Serde<K> keySerde, final >>>>> Serde<V> >>>>>>>>>>>>>>>>>> valueSerde); >>>>>>>>>>>>>>>>>>>>>>>>> One thing I wondered if there way to find out >>>>> topic >>>>>>>>>>>>>>>> related >>>>>>>>>>>>>>>>>> serde >>>>>>>>>>>>>>>>>>>>>> from >>>>>>>>>>>>>>>>>>>>>>>>> TopologyTestDriver topology, it would simply >>>>>> creation >>>>>>>>>>>>>>> of >>>>>>>>>>>>>>>>>> these >>>>>>>>>>>>>>>>>>>> Topic >>>>>>>>>>>>>>>>>>>>>>>>> objects: >>>>>>>>>>>>>>>>>>>>>>>>> public final <K, V> TestInputTopic<K, V> >>>>>>>>>>>>>>>>>>>> createInputTopic(final >>>>>>>>>>>>>>>>>>>>>>>> String >>>>>>>>>>>>>>>>>>>>>>>>> topicName); >>>>>>>>>>>>>>>>>>>>>>>>> public final <K, V> TestOutputTopic<K, V> >>>>>>>>>>>>>>>>>>>> createOutputTopic(final >>>>>>>>>>>>>>>>>>>>>>>> String >>>>>>>>>>>>>>>>>>>>>>>>> topicName); >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> KIP-456 can be discarded if this KIP get >>>>> accepted. >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> Best Regards, >>>>>>>>>>>>>>>>>>>>>>>>> Jukka >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> -- >>>>>>>>>>>>>>>>> -- Guozhang >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> -- >>>>>>>>>>>>>>> -- Guozhang >>>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> >>>>> >>>>> -- >>>>> >>>>> Antony Stubbs >>>>> >>>>> Solutions Architect | Confluent >>>>> >>>>> +44 (0)7491 833 814 <+447491833814> >>>>> Follow us: Twitter <https://twitter.com/ConfluentInc> | blog >>>>> <http://www.confluent.io/blog> >>>>> >>>> >> >
signature.asc
Description: OpenPGP digital signature
