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

Reply via email to