Hi, John
Thanks for pointing that out. I expressed my thoughts about KIP-513 and its
connection to KIP-616 in the KIP-513 mail list.
- Yuriy
On Sun, May 31, 2020 at 1:26 AM John Roesler <vvcep...@apache.org> wrote:
> Hi Yuriy,
>
> I was just looking back at KIP-513, and I’m wondering if there’s any
> overlap we should consider here, or if they are just orthogonal.
>
> Thanks,
> -John
>
> On Thu, May 28, 2020, at 21:36, Yuriy Badalyantc wrote:
> > At the current moment, I think John's plan is better than the original
> plan
> > described in the KIP. I think we should create a new `Serdes` in another
> > package. The old one will be deprecated.
> >
> > - Yuriy
> >
> > On Fri, May 29, 2020 at 8:58 AM John Roesler <vvcep...@apache.org>
> wrote:
> >
> > > Thanks, Matthias,
> > >
> > > If we go with the approach Yuriy and I agreed on, to deprecate and
> replace
> > > the whole class and not just a few of the methods, then the timeline is
> > > less of a concern. Under that plan, Yuriy can just write the new class
> > > exactly the way he wants and people can cleanly swap over to the new
> > > pattern when they are ready.
> > >
> > > The timeline was more significant if we were just going to deprecate
> some
> > > methods and add new methods to the existing class. That plan requires
> two
> > > implementation phases, where we first deprecate the existing methods
> and
> > > later swap the implicits at the same time we remove the deprecated
> members.
> > > Aside from the complexity of that approach, it’s not a breakage free
> path,
> > > as some users would be forced to continue using the deprecated members
> > > until a future release drops them, breaking their source code, and only
> > > then can they update their code.
> > >
> > > That wouldn’t be the end of the world, and we’ve had to do the same
> thing
> > > in the past with the implicit conversations, but this is a much wider
> > > scope, since it’s all the serdes. I’m happy with the new plan, since
> it’s
> > > not only one step, but also it provides everyone a breakage-free path.
> > >
> > > We can still consider dropping the deprecated class in 3.0; I just
> wanted
> > > to clarify how the timeline issue has changed.
> > >
> > > Thanks,
> > > John
> > >
> > > On Thu, May 28, 2020, at 20:34, Matthias J. Sax wrote:
> > > > I am not a Scale person, so I cannot really contribute much. However
> for
> > > > the deprecation period, if we get the change into 2.7, it might be
> ok to
> > > > remove the deprecated classed in 3.0.
> > > >
> > > > It would only be one minor release in between what is a little bit
> short
> > > > (we usually prefer at least two minor released, better three), but
> if we
> > > > have a good reason for it, it might be ok.
> > > >
> > > > If we cannot remove it in 3.0, it seems there would be a 4.0 in
> about a
> > > > year(?) when ZK removal is finished and we can remove the deprecated
> > > > code than.
> > > >
> > > >
> > > > -Matthias
> > > >
> > > > On 5/28/20 7:39 AM, John Roesler wrote:
> > > > > Hi Yuriy,
> > > > >
> > > > > Sounds good to me! I had a feeling we were bringing different
> context
> > > > > to the discussion; thanks for sticking with the conversation until
> we
> > > got
> > > > > it hashed out.
> > > > >
> > > > > I'm glad you prefer Serde*s*, since having multiple different
> classes
> > > with
> > > > > the same name leads to all kinds of trouble. "Serdes" seems
> relatively
> > > > > safe because people in the Scala lib won't be using the Java Serdes
> > > class,
> > > > > and they won't be using the deprecated and non-deprecated one at
> the
> > > > > same time.
> > > > >
> > > > > Thank again,
> > > > > -John
> > > > >
> > > > > On Thu, May 28, 2020, at 02:21, Yuriy Badalyantc wrote:
> > > > >> 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
> > > > >>>>>>>>
> > > > >>>>>>>
> > > > >>>>>>
> > > > >>>>>
> > > > >>>>
> > > > >>>
> > > > >>
> > > >
> > > >
> > > > Attachments:
> > > > * signature.asc
> > >
> >
>
