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
>>>>>>
>>>>>
>>>>
>>>>
>>>
>>
>
signature.asc
Description: OpenPGP digital signature
