Thanks, +1 (binding). On Mon, Sep 3, 2018 at 6:28 AM Viktor Somogyi-Vass <viktorsomo...@gmail.com> wrote:
> Apologies, miscounted the binding votes but the good news is that we need > only one. > > Cheers, > Viktor > > On Mon, Sep 3, 2018 at 11:09 AM Viktor Somogyi-Vass < > viktorsomo...@gmail.com> > wrote: > > > Hi All, > > > > I have completed my binary compatibility testing on this as well. Created > > a small script & example to make this reproducible: > > https://gist.github.com/viktorsomogyi/391defca73e7a46a2c6a40bc699231d4 . > > > > Also, thanks for the votes so far, we're still awaiting for 2 binding. > > > > Cheers, > > Viktor > > > > On Thu, Aug 30, 2018 at 5:09 PM Harsha <ka...@harsha.io> wrote: > > > >> +1. > >> Thanks, > >> Harsha > >> > >> On Thu, Aug 30, 2018, at 4:19 AM, Attila Sasvári wrote: > >> > Thanks for the KIP and the updates Viktor! > >> > > >> > +1 (non-binding) > >> > > >> > > >> > > >> > On Wed, Aug 29, 2018 at 10:44 AM Manikumar <manikumar.re...@gmail.com > > > >> > wrote: > >> > > >> > > +1 (non-binding) > >> > > > >> > > Thanks for the KIP. > >> > > > >> > > On Wed, Aug 29, 2018 at 1:41 AM Jason Gustafson <ja...@confluent.io > > > >> > > wrote: > >> > > > >> > > > +1 Thanks for the updates. > >> > > > > >> > > > On Tue, Aug 28, 2018 at 1:15 AM, Viktor Somogyi-Vass < > >> > > > viktorsomo...@gmail.com> wrote: > >> > > > > >> > > > > Sure, I've added it. I'll also do the testing today. > >> > > > > > >> > > > > On Mon, Aug 27, 2018 at 5:03 PM Ismael Juma <ism...@juma.me.uk> > >> wrote: > >> > > > > > >> > > > > > Thanks Viktor. I think it would be good to verify that > existing > >> > > > > > ExtendedSerializer implementations work without recompiling. > >> This > >> > > could > >> > > > > be > >> > > > > > done as a manual test. If you agree, I suggest adding it to > the > >> > > testing > >> > > > > > plan section. > >> > > > > > > >> > > > > > Ismael > >> > > > > > > >> > > > > > On Mon, Aug 27, 2018 at 7:57 AM Viktor Somogyi-Vass < > >> > > > > > viktorsomo...@gmail.com> > >> > > > > > wrote: > >> > > > > > > >> > > > > > > Thanks guys, I've updated my KIP with this info (so to keep > >> > > solution > >> > > > > #1). > >> > > > > > > If you find it good enough, please vote as well or let me > >> know if > >> > > you > >> > > > > > think > >> > > > > > > something is missing. > >> > > > > > > > >> > > > > > > On Sat, Aug 25, 2018 at 1:14 AM Ismael Juma < > >> ism...@juma.me.uk> > >> > > > wrote: > >> > > > > > > > >> > > > > > > > I'm OK with 1 too. It makes me a bit sad that we don't > have > >> a > >> > > path > >> > > > > for > >> > > > > > > > removing the method without headers, but it seems like the > >> > > simplest > >> > > > > and > >> > > > > > > > least confusing option (I am assuming that headers are not > >> needed > >> > > > in > >> > > > > > the > >> > > > > > > > serializers in the common case). > >> > > > > > > > > >> > > > > > > > Ismael > >> > > > > > > > > >> > > > > > > > On Fri, Aug 24, 2018 at 2:42 PM Jason Gustafson < > >> > > > ja...@confluent.io> > >> > > > > > > > wrote: > >> > > > > > > > > >> > > > > > > > > Hey Viktor, > >> > > > > > > > > > >> > > > > > > > > Good summary. I agree that option 1) seems like the > >> simplest > >> > > > choice > >> > > > > > > and, > >> > > > > > > > as > >> > > > > > > > > you note, we can always add the default implementation > >> later. > >> > > > I'll > >> > > > > > > leave > >> > > > > > > > > Ismael to make a case for the circular forwarding > >> approach ;) > >> > > > > > > > > > >> > > > > > > > > -Jason > >> > > > > > > > > > >> > > > > > > > > On Fri, Aug 24, 2018 at 3:02 AM, Viktor Somogyi-Vass < > >> > > > > > > > > viktorsomo...@gmail.com> wrote: > >> > > > > > > > > > >> > > > > > > > > > I think in the first draft I didn't provide an > >> implementation > >> > > > for > >> > > > > > > them > >> > > > > > > > as > >> > > > > > > > > > it seemed very simple and straightforward. I looked > up a > >> > > couple > >> > > > > of > >> > > > > > > > > > implementations of the ExtendedSerializers on github > >> and the > >> > > > > > general > >> > > > > > > > > > behavior seems to be that they delegate to the 2 > >> argument > >> > > > > > > (headerless) > >> > > > > > > > > > method: > >> > > > > > > > > > > >> > > > > > > > > > > >> https://github.com/khoitnm/practice-spring-kafka-grpc/blob/ > >> > > > > > > > > > > >> a6fc9b3395762c4889807baedd822f4653d5dcdd/kafka-common/src/ > >> > > > > > > > > > > main/java/org/tnmk/common/kafka/serialization/protobuf/ > >> > > > > > > > > > ProtobufSerializer.java > >> > > > > > > > > > > >> > > > > > > > >> > > > > >> https://github.com/hong-zhu/nxgen/blob/5cf1427d4e1a8f5c7fab47955af32a > >> > > > > > > > > > > 0d4f4873af/nxgen-kafka-client/src/main/java/nxgen/kafka/ > >> > > > > > > > > > client/event/serdes/EventSerializer.java > >> > > > > > > > > > https://github.com/jerry-jx/spring-kafka/blob/ > >> > > > > > > > > > > >> ac323ec5b8b9a0ca975db2f7322ff6878fce481a/spring-kafka/src/ > >> > > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > >> > > > > > > > >> > > > > > main/java/org/springframework/kafka/support/serializer/ > >> > > > > JsonSerializer.java > >> > > > > > > > > > > https://github.com/enzobonggio/nonblocking-kafka/blob/ > >> > > > > > > > > > > >> bc1a379b2d9593b46cf9604063bc5b38e2785d19/src/main/java/com/ > >> > > > > > > > > > example/kafka/producer/CustomJsonSerializer.java > >> > > > > > > > > > > >> > > > > > > > > > Of course 4 example is not representative but it shows > >> that > >> > > > these > >> > > > > > > users > >> > > > > > > > > > usually delegate to the "headerless" (2 argument) > >> method. > >> > > I've > >> > > > > > tried > >> > > > > > > to > >> > > > > > > > > > look it up on other code search sites but haven't had > >> much > >> > > luck > >> > > > > so > >> > > > > > > far. > >> > > > > > > > > > Given these examples and the way they implement them > >> I'd say > >> > > > it's > >> > > > > > > more > >> > > > > > > > > > common to delegate to the headerless method, that's > why > >> I > >> > > think > >> > > > > > it's > >> > > > > > > a > >> > > > > > > > > good > >> > > > > > > > > > approach for us too. Now having a default > >> implementation for > >> > > > that > >> > > > > > is > >> > > > > > > > > again > >> > > > > > > > > > a good question. I think current use cases wouldn't > >> change in > >> > > > > > either > >> > > > > > > > case > >> > > > > > > > > > (unless we deprecate the headerless one). > >> > > > > > > > > > For the new use cases it depends what do we want to > >> propagate > >> > > > > going > >> > > > > > > > > > forward. Do we want only one method to exist or two? > As > >> > > Ismael > >> > > > > > > > > highlighted > >> > > > > > > > > > it might be confusing if we have 2 methods, both with > >> default > >> > > > > > > > > > implementation and in this case we want to push the 3 > >> > > argument > >> > > > > one > >> > > > > > > for > >> > > > > > > > > > users. > >> > > > > > > > > > > >> > > > > > > > > > So I see three possible ways: > >> > > > > > > > > > 1.) Don't provide a default implementation for the > >> headerless > >> > > > > > method. > >> > > > > > > > > This > >> > > > > > > > > > supports the current implementations and encourages > the > >> > > > > delegation > >> > > > > > > > style > >> > > > > > > > > in > >> > > > > > > > > > future implementations. This might be the simplest > >> option. > >> > > > > > > > > > 2.) Provide a default implementation for the > headerless > >> > > method. > >> > > > > > This > >> > > > > > > > > would > >> > > > > > > > > > be a bit confusing, so we'd likely push the use of > the 3 > >> > > > > parameter > >> > > > > > > > method > >> > > > > > > > > > and deprecate the headerless. This would however > further > >> > > litter > >> > > > > the > >> > > > > > > > code > >> > > > > > > > > > base with deprecation warnings as we're using the > >> headerless > >> > > > > method > >> > > > > > > in > >> > > > > > > > a > >> > > > > > > > > > lot of places (think of the current > >> > > serializers/deserializers). > >> > > > > So > >> > > > > > in > >> > > > > > > > > this > >> > > > > > > > > > case we would want to clean up the code base a little > >> where > >> > > we > >> > > > > can > >> > > > > > > and > >> > > > > > > > > may > >> > > > > > > > > > remove the headerless method entirely in Kafka 3. But > >> they > >> > > > would > >> > > > > > hang > >> > > > > > > > > > around until that point. I think in this case the > >> > > > implementation > >> > > > > > for > >> > > > > > > > the > >> > > > > > > > > > headerless is a detail question as that is deprecated > >> so we > >> > > > don't > >> > > > > > > > expect > >> > > > > > > > > > new implementations to use that method. > >> > > > > > > > > > If we decide to move this way, we have explored two > >> options > >> > > so > >> > > > > far: > >> > > > > > > > > > returning null / empty array or throwing exceptions. > >> (And I > >> > > > > > honestly > >> > > > > > > > > > started to like the latter as calling that with no > real > >> > > > > > > implementation > >> > > > > > > > is > >> > > > > > > > > > really a programming error.) > >> > > > > > > > > > 3.) We can do it in multiple steps. In the first step > >> we do 1 > >> > > > and > >> > > > > > > later > >> > > > > > > > > 2. > >> > > > > > > > > > I think it would also make sense as the Kafka code > base > >> > > heavily > >> > > > > > uses > >> > > > > > > > the > >> > > > > > > > > > headerless method still (think of the existing > >> > > > > > > > serializers/deserializers) > >> > > > > > > > > > and it would give us time to eliminate/change those > use > >> > > cases. > >> > > > > > > > > > > >> > > > > > > > > > Cheers, > >> > > > > > > > > > Viktor > >> > > > > > > > > > > >> > > > > > > > > > On Thu, Aug 23, 2018 at 11:55 PM Jason Gustafson < > >> > > > > > ja...@confluent.io > >> > > > > > > > > >> > > > > > > > > > wrote: > >> > > > > > > > > > > >> > > > > > > > > > > To clarify, what I am suggesting is to only remove > the > >> > > > default > >> > > > > > > > > > > implementation for these methods. So users would be > >> > > required > >> > > > to > >> > > > > > > > > implement > >> > > > > > > > > > > serialize(topic, data) and deserialize(topic, data). > >> > > > > > > > > > > > >> > > > > > > > > > > -Jason > >> > > > > > > > > > > > >> > > > > > > > > > > On Thu, Aug 23, 2018 at 1:48 PM, Jason Gustafson < > >> > > > > > > ja...@confluent.io > >> > > > > > > > > > >> > > > > > > > > > > wrote: > >> > > > > > > > > > > > >> > > > > > > > > > > > Hey Viktor, > >> > > > > > > > > > > > > >> > > > > > > > > > > > Thinking about it a little more, I wonder if we > >> should > >> > > just > >> > > > > not > >> > > > > > > > > > provide a > >> > > > > > > > > > > > default method for serialize(topic, data) and > >> > > > > > deserialize(topic, > >> > > > > > > > > data). > >> > > > > > > > > > > > Implementing these methods is a trivial burden for > >> users > >> > > > and > >> > > > > it > >> > > > > > > > feels > >> > > > > > > > > > > like > >> > > > > > > > > > > > there's no good solution which allows both methods > >> to > >> > > have > >> > > > > > > default > >> > > > > > > > > > > > implementations. > >> > > > > > > > > > > > > >> > > > > > > > > > > > Also, ack on KIP-331. Thanks for the pointer. > >> > > > > > > > > > > > > >> > > > > > > > > > > > -Jason > >> > > > > > > > > > > > > >> > > > > > > > > > > > On Thu, Aug 23, 2018 at 12:30 PM, Viktor > >> Somogyi-Vass < > >> > > > > > > > > > > > viktorsomo...@gmail.com> wrote: > >> > > > > > > > > > > > > >> > > > > > > > > > > >> Hi Ismael, > >> > > > > > > > > > > >> > >> > > > > > > > > > > >> Regarding the deprecation of the 2 parameter > >> method: > >> > > > should > >> > > > > we > >> > > > > > > do > >> > > > > > > > > this > >> > > > > > > > > > > >> with > >> > > > > > > > > > > >> the Serializer interface as well? > >> > > > > > > > > > > >> > >> > > > > > > > > > > >> I've updated the "Rejected Alternatives" with a > >> few. > >> > > > > > > > > > > >> I've added this circular reference one too but > >> actually > >> > > > > > there's > >> > > > > > > a > >> > > > > > > > > way > >> > > > > > > > > > > >> (pretty heavyweight) by adding a guard class that > >> > > prevents > >> > > > > > > > recursive > >> > > > > > > > > > > >> invocation of either methods. I've tried this out > >> but it > >> > > > > seems > >> > > > > > > to > >> > > > > > > > me > >> > > > > > > > > > an > >> > > > > > > > > > > >> overshoot. So just for the sake of completeness > >> I'll > >> > > copy > >> > > > it > >> > > > > > > here. > >> > > > > > > > > :) > >> > > > > > > > > > > >> > >> > > > > > > > > > > >> public interface Deserializer<T> extends > Closeable > >> { > >> > > > > > > > > > > >> > >> > > > > > > > > > > >> class Guard { > >> > > > > > > > > > > >> > >> > > > > > > > > > > >> private Set<Object> objects = > >> > > > > > > > > Collections.synchronizedSet(new > >> > > > > > > > > > > >> HashSet<>()); // might as well use concurrent > >> hashmap > >> > > > > > > > > > > >> > >> > > > > > > > > > > >> private void methodCallInProgress(Object > >> x) { > >> > > > > > > > > > > >> objects.add(x); > >> > > > > > > > > > > >> } > >> > > > > > > > > > > >> > >> > > > > > > > > > > >> private boolean > >> isMethodCallInProgress(Object > >> > > x) { > >> > > > > > > > > > > >> return objects.contains(x); > >> > > > > > > > > > > >> } > >> > > > > > > > > > > >> > >> > > > > > > > > > > >> private void > >> clearMethodCallInProgress(Object > >> > > x) { > >> > > > > > > > > > > >> objects.remove(x); > >> > > > > > > > > > > >> } > >> > > > > > > > > > > >> > >> > > > > > > > > > > >> private <T> T guard(Supplier<T> > supplier) { > >> > > > > > > > > > > >> if > >> (GUARD.isMethodCallInProgress(this)) { > >> > > > > > > > > > > >> throw new > >> IllegalStateException("You > >> > > must > >> > > > > > > > implement > >> > > > > > > > > > one > >> > > > > > > > > > > of > >> > > > > > > > > > > >> the deserialize methods"); > >> > > > > > > > > > > >> } else { > >> > > > > > > > > > > >> try { > >> > > > > > > > > > > >> > >> GUARD.methodCallInProgress(this); > >> > > > > > > > > > > >> return supplier.get(); > >> > > > > > > > > > > >> } finally { > >> > > > > > > > > > > >> > >> > > GUARD.clearMethodCallInProgress(this); > >> > > > > > > > > > > >> } > >> > > > > > > > > > > >> } > >> > > > > > > > > > > >> } > >> > > > > > > > > > > >> } > >> > > > > > > > > > > >> > >> > > > > > > > > > > >> Guard GUARD = new Guard(); > >> > > > > > > > > > > >> > >> > > > > > > > > > > >> void configure(Map<String, ?> configs, > boolean > >> > > isKey); > >> > > > > > > > > > > >> > >> > > > > > > > > > > >> default T deserialize(String topic, byte[] > >> data) { > >> > > > > > > > > > > >> return GUARD.guard(() -> > deserialize(topic, > >> > > null, > >> > > > > > > data)); > >> > > > > > > > > > > >> } > >> > > > > > > > > > > >> > >> > > > > > > > > > > >> default T deserialize(String topic, Headers > >> headers, > >> > > > > > byte[] > >> > > > > > > > > data) > >> > > > > > > > > > { > >> > > > > > > > > > > >> return GUARD.guard(() -> > deserialize(topic, > >> > > > data)); > >> > > > > > > > > > > >> } > >> > > > > > > > > > > >> > >> > > > > > > > > > > >> @Override > >> > > > > > > > > > > >> void close(); > >> > > > > > > > > > > >> } > >> > > > > > > > > > > >> > >> > > > > > > > > > > >> > >> > > > > > > > > > > >> Cheers, > >> > > > > > > > > > > >> Viktor > >> > > > > > > > > > > >> > >> > > > > > > > > > > >> On Thu, Aug 23, 2018 at 3:50 PM Ismael Juma < > >> > > > > > ism...@juma.me.uk> > >> > > > > > > > > > wrote: > >> > > > > > > > > > > >> > >> > > > > > > > > > > >> > Also, we may consider deprecating the > deserialize > >> > > method > >> > > > > > that > >> > > > > > > > does > >> > > > > > > > > > not > >> > > > > > > > > > > >> take > >> > > > > > > > > > > >> > headers. Yes, it's a convenience, but it also > >> adds > >> > > > > > confusion. > >> > > > > > > > > > > >> > > >> > > > > > > > > > > >> > Ismael > >> > > > > > > > > > > >> > > >> > > > > > > > > > > >> > On Thu, Aug 23, 2018 at 6:48 AM Ismael Juma < > >> > > > > > > ism...@juma.me.uk> > >> > > > > > > > > > > wrote: > >> > > > > > > > > > > >> > > >> > > > > > > > > > > >> > > I think the KIP needs the rejected > alternatives > >> > > > section > >> > > > > to > >> > > > > > > > have > >> > > > > > > > > > more > >> > > > > > > > > > > >> > > detail. For example, another option would be > >> > > something > >> > > > > > like > >> > > > > > > > the > >> > > > > > > > > > > >> > following, > >> > > > > > > > > > > >> > > which works great as long as one overrides > one > >> of > >> > > the > >> > > > > > > methods, > >> > > > > > > > > but > >> > > > > > > > > > > >> pretty > >> > > > > > > > > > > >> > > bad if one doesn't. :) > >> > > > > > > > > > > >> > > > >> > > > > > > > > > > >> > > default T deserialize(String topic, byte[] > >> data) { > >> > > > > > > > > > > >> > > return deserialize(topic, null, data); > >> > > > > > > > > > > >> > > } > >> > > > > > > > > > > >> > > > >> > > > > > > > > > > >> > > default T deserialize(String topic, Headers > >> headers, > >> > > > > > byte[] > >> > > > > > > > > data) > >> > > > > > > > > > { > >> > > > > > > > > > > // > >> > > > > > > > > > > >> > > This is the new method > >> > > > > > > > > > > >> > > return deserialize(topic, data); > >> > > > > > > > > > > >> > > } > >> > > > > > > > > > > >> > > > >> > > > > > > > > > > >> > > > >> > > > > > > > > > > >> > > On Thu, Aug 23, 2018 at 3:57 AM Viktor > >> Somogyi-Vass > >> > > < > >> > > > > > > > > > > >> > > viktorsomo...@gmail.com> wrote: > >> > > > > > > > > > > >> > > > >> > > > > > > > > > > >> > >> Hi Jason, > >> > > > > > > > > > > >> > >> > >> > > > > > > > > > > >> > >> Thanks for the feedback. > >> > > > > > > > > > > >> > >> 1. I chose to return null here because > >> according to > >> > > > the > >> > > > > > > > > > > >> documentation it > >> > > > > > > > > > > >> > >> may return null data, therefore the users of > >> this > >> > > > > methods > >> > > > > > > are > >> > > > > > > > > > > >> perpared > >> > > > > > > > > > > >> > for > >> > > > > > > > > > > >> > >> getting a null. Thinking of it though it may > >> be > >> > > > better > >> > > > > to > >> > > > > > > > throw > >> > > > > > > > > > an > >> > > > > > > > > > > >> > >> exception by default because it'd indicate a > >> > > > > programming > >> > > > > > > > error. > >> > > > > > > > > > > >> However, > >> > > > > > > > > > > >> > >> would that be a backward incompatible > change? > >> I'm > >> > > > > simply > >> > > > > > > > > thinking > >> > > > > > > > > > > of > >> > > > > > > > > > > >> > this > >> > > > > > > > > > > >> > >> because this is a new behavior that we'd > >> introduce > >> > > > but > >> > > > > > I'm > >> > > > > > > > not > >> > > > > > > > > > sure > >> > > > > > > > > > > >> yet > >> > > > > > > > > > > >> > if > >> > > > > > > > > > > >> > >> it'd cause problems. > >> > > > > > > > > > > >> > >> Do you think it'd make sense to do the same > in > >> > > > > > `serialize`? > >> > > > > > > > > > > >> > >> 2. Yes, I believe that is covered in > KIP-331: > >> > > > > > > > > > > >> > >> > >> > > > > > > > > > > >> > >> > >> > > > > > > > > > > >> > > >> > > https://cwiki.apache.org/confluence/display/KAFKA/KIP- > >> > > > > 331+ > >> > > > > > > > > > > >> > >> > > > Add+default+implementation+to+close%28%29+and+configure%28% > >> > > > > > > > > > > >> 29+for+Serializer%2C+Deserializer+and+Serde > >> > > > > > > > > > > >> > >> > >> > > > > > > > > > > >> > >> Cheers, > >> > > > > > > > > > > >> > >> Viktor > >> > > > > > > > > > > >> > >> > >> > > > > > > > > > > >> > >> On Wed, Aug 22, 2018 at 6:11 PM Jason > >> Gustafson < > >> > > > > > > > > > > ja...@confluent.io> > >> > > > > > > > > > > >> > >> wrote: > >> > > > > > > > > > > >> > >> > >> > > > > > > > > > > >> > >> > Hey Viktor, > >> > > > > > > > > > > >> > >> > > >> > > > > > > > > > > >> > >> > This is a nice cleanup. Just a couple > quick > >> > > > > questions: > >> > > > > > > > > > > >> > >> > > >> > > > > > > > > > > >> > >> > 1. Rather than returning null for the > >> default > >> > > > > > > > > > `deserialize(topic, > >> > > > > > > > > > > >> > >> data)`, > >> > > > > > > > > > > >> > >> > would it be better to throw > >> > > > > > > UnsupportedOperationException? > >> > > > > > > > I > >> > > > > > > > > > > assume > >> > > > > > > > > > > >> > that > >> > > > > > > > > > > >> > >> > internally we'll always invoke the api > which > >> > > takes > >> > > > > > > headers. > >> > > > > > > > > > > >> Similarly > >> > > > > > > > > > > >> > >> for > >> > > > > > > > > > > >> > >> > `serialize(topic, data)`. > >> > > > > > > > > > > >> > >> > 2. Would it make sense to have default > no-op > >> > > > > > > > implementations > >> > > > > > > > > > for > >> > > > > > > > > > > >> > >> > `configure` and `close`? > >> > > > > > > > > > > >> > >> > > >> > > > > > > > > > > >> > >> > Thanks, > >> > > > > > > > > > > >> > >> > Jason > >> > > > > > > > > > > >> > >> > > >> > > > > > > > > > > >> > >> > On Wed, Aug 22, 2018 at 5:27 AM, Satish > >> Duggana < > >> > > > > > > > > > > >> > >> satish.dugg...@gmail.com> > >> > > > > > > > > > > >> > >> > wrote: > >> > > > > > > > > > > >> > >> > > >> > > > > > > > > > > >> > >> > > +1 > >> > > > > > > > > > > >> > >> > > > >> > > > > > > > > > > >> > >> > > On Wed, Aug 22, 2018 at 4:45 PM, Ted Yu > < > >> > > > > > > > > yuzhih...@gmail.com > >> > > > > > > > > > > > >> > > > > > > > > > > >> > wrote: > >> > > > > > > > > > > >> > >> > > > >> > > > > > > > > > > >> > >> > > > +1 > >> > > > > > > > > > > >> > >> > > > -------- Original message > --------From: > >> Kamal > >> > > > > > > > > > Chandraprakash > >> > > > > > > > > > > < > >> > > > > > > > > > > >> > >> > > > kamal.chandraprak...@gmail.com> Date: > >> > > 8/22/18 > >> > > > > > 3:19 > >> > > > > > > AM > >> > > > > > > > > > > >> > (GMT-08:00) > >> > > > > > > > > > > >> > >> > To: > >> > > > > > > > > > > >> > >> > > > dev@kafka.apache.org Subject: Re: > >> [VOTE] > >> > > > > KIP-336: > >> > > > > > > > > > > Consolidate > >> > > > > > > > > > > >> > >> > > > ExtendedSerializer/Serializer and > >> > > > > > > > > > > >> > ExtendedDeserializer/Deserializer > >> > > > > > > > > > > >> > >> > > > +1 > >> > > > > > > > > > > >> > >> > > > > >> > > > > > > > > > > >> > >> > > > Thanks for the KIP! > >> > > > > > > > > > > >> > >> > > > > >> > > > > > > > > > > >> > >> > > > On Wed, Aug 22, 2018 at 2:48 PM Viktor > >> > > > > > Somogyi-Vass < > >> > > > > > > > > > > >> > >> > > > viktorsomo...@gmail.com> > >> > > > > > > > > > > >> > >> > > > wrote: > >> > > > > > > > > > > >> > >> > > > > >> > > > > > > > > > > >> > >> > > > > Hi All, > >> > > > > > > > > > > >> > >> > > > > > >> > > > > > > > > > > >> > >> > > > > I'd like to start a vote on this > KIP ( > >> > > > > > > > > > > >> > >> > > > > > >> > > > > > https://cwiki.apache.org/confluence/pages/viewpage > >> > > > > > > . > >> > > > > > > > > > > >> > >> > > > action?pageId=87298242) > >> > > > > > > > > > > >> > >> > > > > which aims to refactor > >> > > > > > > ExtendedSerializer/Serializer > >> > > > > > > > > and > >> > > > > > > > > > > >> > >> > > > > ExtendedDeserializer/Deserializer. > >> > > > > > > > > > > >> > >> > > > > > >> > > > > > > > > > > >> > >> > > > > To summarize what's the motivation: > >> > > > > > > > > > > >> > >> > > > > > >> > > > > > > > > > > >> > >> > > > > When headers were introduced by > >> KIP-82 the > >> > > > > > > > > > > ExtendedSerializer > >> > > > > > > > > > > >> > and > >> > > > > > > > > > > >> > >> > > > > ExtendedDeserializer classes were > >> created > >> > > in > >> > > > > > order > >> > > > > > > to > >> > > > > > > > > > keep > >> > > > > > > > > > > >> > >> interface > >> > > > > > > > > > > >> > >> > > > > compatibility but still add `T > >> > > > > deserialize(String > >> > > > > > > > > topic, > >> > > > > > > > > > > >> Headers > >> > > > > > > > > > > >> > >> > > headers, > >> > > > > > > > > > > >> > >> > > > > byte[] data);` and `byte[] > >> serialize(String > >> > > > > > topic, > >> > > > > > > > > > Headers > >> > > > > > > > > > > >> > >> headers, T > >> > > > > > > > > > > >> > >> > > > > data);` methods that consume the > >> headers > >> > > for > >> > > > > > > > > > > >> > >> > > > serialization/deserialization. > >> > > > > > > > > > > >> > >> > > > > The reason for doing so was that > >> Kafka at > >> > > > that > >> > > > > > time > >> > > > > > > > > > needed > >> > > > > > > > > > > be > >> > > > > > > > > > > >> > >> > > compatbile > >> > > > > > > > > > > >> > >> > > > > with Java 7. Since we're not > >> compiling on > >> > > > Java > >> > > > > 7 > >> > > > > > > > > anymore > >> > > > > > > > > > > >> > >> (KAFKA-4423) > >> > > > > > > > > > > >> > >> > > > we'll > >> > > > > > > > > > > >> > >> > > > > try consolidate the way we're using > >> these > >> > > in > >> > > > a > >> > > > > > > > backward > >> > > > > > > > > > > >> > compatible > >> > > > > > > > > > > >> > >> > > > fashion: > >> > > > > > > > > > > >> > >> > > > > deprecating the Extended* classes > and > >> > > moving > >> > > > > the > >> > > > > > > > > > > >> aforementioned > >> > > > > > > > > > > >> > >> > methods > >> > > > > > > > > > > >> > >> > > > up > >> > > > > > > > > > > >> > >> > > > > in the class hierarchy. > >> > > > > > > > > > > >> > >> > > > > > >> > > > > > > > > > > >> > >> > > > > I'd be happy to get votes or > >> additional > >> > > > > feedback > >> > > > > > on > >> > > > > > > > > this. > >> > > > > > > > > > > >> > >> > > > > > >> > > > > > > > > > > >> > >> > > > > Viktor > >> > > > > > > > > > > >> > >> > > > > > >> > > > > > > > > > > >> > >> > > > > >> > > > > > > > > > > >> > >> > > > >> > > > > > > > > > > >> > >> > > >> > > > > > > > > > > >> > >> > >> > > > > > > > > > > >> > > > >> > > > > > > > > > > >> > > >> > > > > > > > > > > >> > >> > > > > > > > > > > > > >> > > > > > > > > > > > > >> > > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > >> > > > > > > > >> > > > > > > >> > > > > > >> > > > > >> > > > >> > > >
