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
> > > >
> > >
> >
>
