Ok, I understood you, John. I wasn't sure about kafka deprecation policy and thought that the full cycle could be done with 2.7 version. Waiting for 3.0 is too much, I agree with it.
So, I think creating one more `Serdes` in another package is our way. I suggest one of the following: 1. `org.apache.kafka.streams.scala.serde.Serdes` 2. `org.apache.kafka.streams.scala.serialization.Serdes` About `Serde` vs `Serdes`. I'm strongly against `Serde` because it would lead to a new name clash with the `org.apache.kafka.common.serialization.Serde`. - Yuriy On Thu, May 28, 2020 at 11:12 AM John Roesler <vvcep...@apache.org> wrote: > Hi Yuriy, > > Thanks for the clarification. > > I guess my concern is twofold: > 1. We typically leave deprecated methods in place for at least a major > release cycle before removing them, so it would seem abrupt to have a > deprecation period of only one minor release. If we follow the same pattern > here, it would take over a year to finish this KIP. > 2. It doesn’t seem like there is a nonbreaking deprecation path at all if > people enumerate their imports (if they don’t use a wildcard). In that > case, they would have no path to implicitly use the newly named serdes, and > therefore they would have no way to avoid continuing to use the deprecated > ones. > > Since you mentioned that your reason is mainly the preference for the name > “Serde” or “Serdes”, can we explore just using one of those? Would it cause > some kind of conflict to use org.apache.kafka.streams.scala.Serde or to use > Serdes in a different package, like > org.apache.kafka.streams.scala.implicit.Serdes? > > I empathize with this desire. I faced the same dilemma when I wanted to > replace Processor but keep the class name in KIP-478. I wound up creating a > new package for the new Processor. > > Thanks, > John > > On Wed, May 27, 2020, at 22:20, Yuriy Badalyantc wrote: > > Hi John, > > > > I'm stick with the `org.apache.kafka.streams.scala.Serdes` because it's > > sort of conventional in the scala community. If you have a typeclass > `Foo`, > > you probably will search `Foo` related stuff in the `Foo` or maybe `Foos` > > (plural). All other places are far less discoverable for the developers. > > > > I agree that the migration path is a bit complex for such change. But I > > think it's more important to provide good developer experience than to > > simplify migration. Also, I think it's debatable which migration path is > > better for library users. If we would create, for example, `Serdes2`, > > library users will have to modify their code if they used any part of the > > old `Serde`. With my approach, most of the old code will still work > without > > changes. Only explicit usage of implicits will need to be fixed (because > > names will be changed, and old names will be deprecated). Wildcard > imports > > will work without changes and will not lead to a name clash. Moreover, > many > > users may not notice name clash problems. And with my migration path, > they > > will not notice any changes at all. > > > > - Yuriy > > > > On Thu, May 28, 2020 at 7:48 AM John Roesler <vvcep...@apache.org> > wrote: > > > > > Hi Yuriy, > > > > > > Thanks for the reply. I guess I've been out of the Scala game for a > > > while; all this summoner business is totally new to me. > > > > > > I think I followed the rationale you provided, but I still don't see > > > why you can't implement your whole plan in a new class. What > > > is special about the existing Serdes class? > > > > > > Thanks, > > > -John > > > > > > On Tue, May 19, 2020, at 01:18, Yuriy Badalyantc wrote: > > > > Hi John, > > > > > > > > Your suggestion looks interesting. I think it's technically doable. > But > > > I'm > > > > not sure that this is the better solution. I will try to explain. > From > > > the > > > > scala developers' perspective, `Serde` looks really like a typeclass. > > > > Typical typeclass in pure scala will look like this: > > > > > > > > ``` > > > > trait Serde[A] { > > > > def serialize(data: A): Array[Byte] > > > > def deserialize(data: Array[Byte]): A > > > > } > > > > object Serde extends DefaultSerdes { > > > > // "summoner" function. With this I can write `Serde[A]` and this > serde > > > > will be implicitly summonned. > > > > def apply[A](implicit ev: Serde[A]): Serde[A] = ev > > > > } > > > > > > > > trait DefaultSerdes { > > > > // default instances here > > > > } > > > > ``` > > > > > > > > Usage example (note, that there are no wildcards imports here): > > > > ``` > > > > object Main extends App { > > > > import Serde // not wildcard import > > > > > > > > // explicit summonning: > > > > val stringSerde = Serde[String] // using summoner > > > > stringSerde.serialize(???) > > > > > > > > // implicit summonning > > > > def serialize[A: Serde](a: A) = { > > > > Serde[A].serialize(a) // summoner again > > > > } > > > > serialize("foo") > > > > } > > > > ``` > > > > > > > > Examples are pretty silly, but I just want to show common patterns of > > > > working with typeclasses in scala. All default instances in the usage > > > > examples are found using implicits searching mechanism. Scala > compiler > > > > searches implicits in a lot of places. Including companion objects. > In my > > > > examples compiler will found `Serde[String]` instance in the > companion > > > > object of `Serde` typeclass. > > > > > > > > Also, I want to pay attention to the summoner function. It makes > usage of > > > > typeclasses very neat and clear. > > > > > > > > The example above was the example of the perfect solution for the > scala > > > > developers. But this solution requires to create separate `Serde` > > > > typeclass, to make all this implicit searching stuff works. I don't > think > > > > that it worth it, because a lot of code should be reimplemented using > > > this > > > > new typeclass. But the main point of my example is to show the > perfect > > > > solution. And I think we should strive to provide developer > experience > > > > close to this. > > > > > > > > It's a bit out of the scope of my KIP, but I have a plan to make > > > > `org.apache.kafka.streams.scala.Serdes` more closer to the solution > > > above. > > > > It could be done in 2 steps: > > > > 1. Fix implicit names. > > > > 2. Add summoner function. > > > > > > > > And with this scala developers will be able to write almost the same > code > > > > as in the example above: > > > > ``` > > > > object Main extends App { > > > > import org.apache.kafka.streams.scala.Serdes // not wildcard import > > > > > > > > val stringSerde = Serdes[String] > > > > stringSerde.serialize(???) > > > > > > > > def serialize[A: Serde](a: A) = { > > > > Serdes[A].serialize(a) > > > > } > > > > serialize("foo") > > > > } > > > > ``` > > > > Of course, wildcard import will still work. > > > > > > > > Other names will make this new entity (containing default implicits) > less > > > > discoverable. And summoner usage, in this case, will look weird: > > > > ``` > > > > object Main extends App { > > > > import org.apache.kafka.streams.scala.DefaultSerdes // not wildcard > > > import > > > > > > > > val stringSerde = DefaultSerdes[String] > > > > stringSerde.serialize(???) > > > > > > > > def serialize[A: Serde](a: A) = { > > > > DefaultSerdes[A].serialize(a) > > > > } > > > > serialize("foo") > > > > } > > > > ``` > > > > > > > > So, I think it's more important to provide a solid and familiar > developer > > > > experience for the scala developer. And renaming (or creating a new > > > > version) of `Serdes` will not help here. > > > > > > > > -Yuriy > > > > > > > > On Tue, May 19, 2020 at 11:56 AM John Roesler <vvcep...@apache.org> > > > wrote: > > > > > > > > > Hi Yuriy, > > > > > > > > > > Thanks so much for the KIP! I didn’t anticipate the problem you > laid > > > out > > > > > in the KIP, but I find it very plausible. > > > > > > > > > > Thanks for pushing back on the “convention” and raising the issue, > and > > > > > also volunteering a solution! > > > > > > > > > > I’m wondering if we can “fix” it in one shot by just deprecating > the > > > whole > > > > > Serdes class and replacing it with a new one containing the defs > you > > > > > proposed. Then, people could just switch their import to the new > one. > > > > > > > > > > Of course the new class needs to have a different name, which is > > > always a > > > > > challenge in situations like this, so I might just throw out > > > ImplicitSerdes > > > > > as an option. > > > > > > > > > > Do you think this would work? > > > > > > > > > > Thanks again, > > > > > John > > > > > > > > > > On Mon, May 18, 2020, at 23:35, Yuriy Badalyantc wrote: > > > > > > Hi, > > > > > > > > > > > > I would like to propose KIP-616 to fix naming clash in the kafka > > > > > > streams scala API: > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-616%3A+Rename+implicit+Serdes+instances+in+kafka-streams-scala > > > > > > > > > > > > Looking forward to your feedback. > > > > > > > > > > > > -Yuriy > > > > > > > > > > > > > > > > > > > > >