[ https://issues.apache.org/jira/browse/KAFKA-12313?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17303843#comment-17303843 ]
Sagar Rao edited comment on KAFKA-12313 at 3/18/21, 4:08 AM: ------------------------------------------------------------- Haha my bad i should have been more elaborate about "it" :D. Your suggestions make perfect sense to me. Summarising them one final time here(quoting your rules from the last comment) : # Deprecate inner class key/value configs. Replace with one deserialiser config (windowed.deserializer.inner.class) which basically represents a WindowedKey. # {noformat} For the console-consumer, you have to pass in both parameters via the configs{noformat} Console consumer to invoke the default constructor of TimeWindowedDeserializer and we will ensure that both parameters(Deserialiser and windowSize) configs are passed. # {noformat} For any other plain consumer client, you can pass them in as configs OR pass the parameters to the TimeWindowedDeserializer construct, and then pass that object to the consumer. I guess it doesn't hurt if a user supplies BOTH the configs and an actual Deserializer, as long as the parameters match{noformat} A normal consumer client can use either of the 2 constructors. We have to ensure that both the deserialiser and windowsize are set and there are no conflicts amongst them(similar to how windowSize is being validated). # {noformat} For use in Kafka Streams (such as the DSL), you must supply the parameters by constructing a TimeWindowedSerde and passing that in as a parameter to any relevant DSL operators{noformat} Here I believe the way the TemperatureDemo class creates the WindowSerde is the way that users can use it in the DSL. {code:java} final Serde<Windowed<String>> windowedSerde = WindowedSerdes.timeWindowedSerdeFrom(String.class, TEMPERATURE_WINDOW_SIZE);{code} So, this is already being taken care of so I don't expect any changes here. If you can confirm these, then I can start with the next steps. BTW, the label on the ticket says `needs-kip`. If the summary above looks fine to you, then should I go ahead with the KIP creation? was (Author: sagarrao): Haha my bad i should have been more elaborate about "it" :D. Your suggestions make perfect sense to me. Summarising them one final time here(quoting your rules from the last comment) : # Deprecate inner class key/value configs. Replace with one deserialiser config (windowed.deserializer.inner.class) which basically represents a WindowedKey. # {noformat} For the console-consumer, you have to pass in both parameters via the configs{noformat} Console consumer to invoke the default constructor of TimeWindowedDeserializer and we will ensure that both parameters(Deserialiser and windowSize) configs are passed. # {noformat} For any other plain consumer client, you can pass them in as configs OR pass the parameters to the TimeWindowedDeserializer construct, and then pass that object to the consumer. I guess it doesn't hurt if a user supplies BOTH the configs and an actual Deserializer, as long as the parameters match{noformat} A normal consumer client can use either of the 2 constructors. We have to ensure that both the deserialiser and windowsize are set and there is no conflicts amongst them(similar to how windowSize is being validated). # {noformat} For use in Kafka Streams (such as the DSL), you must supply the parameters by constructing a TimeWindowedSerde and passing that in as a parameter to any relevant DSL operators{noformat} Here I believe the way the TemperatureDemo class creates the WindowSerde is the way that users can use it in the DSL. {code:java} final Serde<Windowed<String>> windowedSerde = WindowedSerdes.timeWindowedSerdeFrom(String.class, TEMPERATURE_WINDOW_SIZE);{code} So, this is already being taken care of so I don't expect any changes here. If you can confirm these, then I can start with the next steps. BTW, the label on the ticket says `needs-kip`. If the summary above looks fine to you, then should I go ahead with the KIP creation? > Consider deprecating the default.windowed.serde.inner.class configs > ------------------------------------------------------------------- > > Key: KAFKA-12313 > URL: https://issues.apache.org/jira/browse/KAFKA-12313 > Project: Kafka > Issue Type: Improvement > Components: streams > Reporter: A. Sophie Blee-Goldman > Assignee: Sagar Rao > Priority: Major > Labels: needs-kip > Fix For: 3.0.0 > > > During the discussion of KIP-659 we discussed whether it made sense to have a > "default" class for the serdes of windowed inner classes across Streams. > Using these configs instead of specifying an actual Serde object can lead to > subtle bugs, since the WindowedDeserializer requires a windowSize in addition > to the inner class. If the default constructor is invoked, as it will be when > falling back on the config, this windowSize defaults to MAX_VALUE. > If the downstream program doesn't care about the window end time in the > output, then this can go unnoticed and technically there is no problem. But > if anything does depend on the end time, or the user just wants to manually > read the output for testing purposes, then the MAX_VALUE will result in a > garbage timestamp. > We should consider whether the convenience of specifying a config instead of > instantiating a Serde in each operator is really worth the risk of a user > accidentally failing to specify a windowSize -- This message was sent by Atlassian Jira (v8.3.4#803005)