Thanks for taking the time to really fill in the background details for
this KIP.
The Motivation section is very informative. Hopefully this will also serve
as a
warning against making similar such mistakes in the future :P

I notice that the `Window` class that
parametrizes EnumerableWindowDefinition
is also abstract. Did you consider migrating that to an interface as well?

Also, pretty awesome that we can solve the issue with varying fixed sized
windows
(eg calendar months) on the side. For users who may have already extended
Windows,
do you plan to just have Windows implement the new #maxSize method and
return the existing
size until Windows gets removed?

One final note: this seems to be implicitly implied throughout the KIP but
just to be clear,
you will be replacing any DSL methods that accept Windows with identical
DSL methods
that take the new EnumerableWindowDefinition as argument. So there is
nothing deprecated
and nothing added, but there are public methods changed. Is that right?

On Sun, Jul 26, 2020 at 1:23 PM John Roesler <vvcep...@apache.org> wrote:

> Thanks Sophie and Boyang for asking for specifics.
>
> As far as I can tell, if we were to _remove_ all the non-public-method
> members from Windows, including any constructors, we are left with
> effectively an interface. I think this was my plan before. I don't think
> I realized at the time that it's possible to replace the entire class with
> an interface. Now I realize it is possible, hence the motivation to do it.
>
> We can choose either to maintain forever the tech debt of having to
> enforce that Windows look, sound, and act just like an interface, or we
> can just replace it with an interface and be done with it. I.e., the
> motivation is less maintenence burden for us and for users.
>
> Coincidentally, I had an interesting conversation with Matthias about
> this interface, and he made me realize that "fixed size" isn't the
> essential
> nature of this entity. Instead being enumerable is. Reframing the interface
> in this way will enable us to implement variable sized windows like
> MonthlyWindows.
>
> So, now there are two good reasons to vote for this KIP :)
>
> Anyway, I've updated the proposed interface and the motivation.
>
> To Sophie latter question, all of the necessary deprecation is listed
> in the KIP. We do not have to deprecate any windowBy methods.
>
> Thanks,
> -John
>
> On Sat, Jul 25, 2020, at 00:52, Boyang Chen wrote:
> > Thanks for the KIP John. I share a similar concern with the motivation,
> it
> > would be good if you could cast light on the actual downside of using a
> > base class vs the interface, is it making the code fragile, or requiring
> > redundant implementation, etc.
> >
> > Boyang
> >
> > On Tue, Jul 21, 2020 at 2:19 PM Sophie Blee-Goldman <sop...@confluent.io
> >
> > wrote:
> >
> > > Hey John,
> > >
> > > Thanks for the KIP. I know this has been bugging you :)
> > >
> > > That said, I think the KIP is missing some elaboration in the
> Motivation
> > > section.
> > > You mention a number of problems we've had and lived with in the past
> --
> > > could
> > > you give an example of one, and how it would be solved by your
> proposal?
> > >
> > > By the way, I assume we would also need to deprecate all APIs that
> accept a
> > > Windows
> > > parameter in favor of new ones that accept a
> FixedSizeWindowDefinition? Off
> > > the
> > > top of my head that would be the windowedBy methods in KGroupedStream
> and
> > > CogroupedKStream
> > >
> > > On Tue, Jul 21, 2020 at 1:46 PM John Roesler <vvcep...@apache.org>
> wrote:
> > >
> > > > Hello all,
> > > >
> > > > I'd like to propose KIP-645, to correct a small API mistake in
> Streams.
> > > > Fixing this now allows us to avoid perpetuating the mistake in new
> work.
> > > > For example, it will allow us to implement KIP-450 cleanly.
> > > >
> > > > The change itself should be seamless for users.
> > > >
> > > > Please see https://cwiki.apache.org/confluence/x/6SN4CQ for details.
> > > >
> > > > Thanks,
> > > > -John
> > > >
> > >
> >
>

Reply via email to