If anyone wants to review the KIP call discussion we had on this just
before the vote, here is a link to the relevant session (6 minutes in):
https://youtu.be/Hcjur17TjBE?t=6m

On Fri, Apr 29, 2016 at 12:21 PM, Grant Henke <ghe...@cloudera.com> wrote:

> I think you are right Jason. People were definitely on the fence about
> this and we went back and forth for quite some time.
>
> I think the main point in the KIP discussion that made this decision, is
> that the Consumer was annotated with the Unstable annotation. Given how
> new the Consumer is, we wanted to leverage that to make sure the interface
> is clean. The same will be true for KafkaStreams in the upcoming release.
>
> We did agree that we should discuss the annotations and what our
> compatibility story is in the future. However, for now the documentation of
> the Unstable annotation says, "No guarantee is provided as to reliability
> or stability across any level of release granularity."  If we can't
> leverage the Unstable annotation to make breaking changes where necessary,
> it will be tough to vet new apis without generating a lot of deprecated
> code.
>
> Note: We did remove the Unstable annotation from the Consumer interface
> for 0.10 implying that it is now stable. (KAFKA-3435
> <https://issues.apache.org/jira/browse/KAFKA-3435>)
>
> Thanks,
> Grant
>
> On Fri, Apr 29, 2016 at 12:05 PM, Jason Gustafson <ja...@confluent.io>
> wrote:
>
>> Hey Harsha,
>>
>> One issue with adding back subscribe(List), but marking it deprecated is
>> that it may confuse some users if they use the typical Arrays.asList()
>> pattern. You'd have to cast to a Collection to avoid the deprecation
>> warning, which is awkward. Maybe it would be better in that case to keep
>> the List alternatives forever?
>>
>> In general, I'm not opposed to adding the methods back. When we voted on
>> KIP-45, I think many of us were on the fence anyway. It would be nice to
>> hear what others think.
>>
>> -Jason
>>
>> On Thu, Apr 28, 2016 at 6:30 PM, Harsha <ka...@harsha.io> wrote:
>>
>> > Hi Jason,
>> >     "t. I think what you're
>> > saying is that the KafkaSpout has been compiled against the 0.9 client,
>> > but
>> > it may need to be to run against 0.10 (if the user depends on that
>> > version
>> > instead). Is that correct?"
>> >
>> > Yes thats true.
>> >
>> > " Is that correct? In general, are you expecting that KafkaSpout
>> > > > will work with any kafka-clients greater than 0.9?"
>> >
>> > In general yes . But given that interface is marked unstable its
>> > probably not reasonable to expect to work across the new versions
>> > of the Kafka.
>> >
>> > "Another question that
>> > > > comes to mind is whether we would also need to revert to the old
>> > versions
>> > > > of subscribe() and assign().
>> > Yes you are right on these methods. We need to add for these two as
>> > well.
>> >
>> >
>> > My issue is users who built their clients using 0.9.x java api will have
>> > to change once the 0.10 release is out. Alternative I am proposing is to
>> > give these users time to move onto the new api thats added and keep the
>> > old methods with deprecated tag for atleast one version.
>> >
>> > Thanks,
>> > Harsha
>> >
>> >
>> > On Thu, Apr 28, 2016, at 04:41 PM, Grant Henke wrote:
>> > > FYI. I have attached a sample clients API change/compatibility report
>> to
>> > > KAFKA-1880 <https://issues.apache.org/jira/browse/KAFKA-1880>. The
>> > report
>> > > shows changes in the public apis between the 0.9 and trunk branches.
>> Some
>> > > of them are expected per KIP-45 obviously.
>> > >
>> > > Thanks,
>> > > Grant
>> > >
>> > >
>> > > On Thu, Apr 28, 2016 at 6:33 PM, Jason Gustafson <ja...@confluent.io>
>> > > wrote:
>> > >
>> > > > Hey Harsha,
>> > > >
>> > > > We're just trying to understand the problem first. I think what
>> you're
>> > > > saying is that the KafkaSpout has been compiled against the 0.9
>> > client, but
>> > > > it may need to be to run against 0.10 (if the user depends on that
>> > version
>> > > > instead). Is that correct? In general, are you expecting that
>> > KafkaSpout
>> > > > will work with any kafka-clients greater than 0.9? Another question
>> > that
>> > > > comes to mind is whether we would also need to revert to the old
>> > versions
>> > > > of subscribe() and assign(). The argument type was changed from
>> List to
>> > > > Collection, which is not binary compatible, right?
>> > > >
>> > > > Thanks,
>> > > > Jason
>> > > >
>> > > > On Thu, Apr 28, 2016 at 1:41 PM, Harsha <ka...@harsha.io> wrote:
>> > > >
>> > > > > Hi Ismael,
>> > > > >               This will solve both binary and source
>> compatibility.
>> > > > >               Storm has new KafkaSpout that used 0.9.x new
>> KafkaSpout
>> > > > >               API. As part of that spout we used
>> > > > >               KafkaConsumer.seekToBeginning and other methods.
>> Since
>> > the
>> > > > >               method signature changed as part of KIP-45. If we
>> > update
>> > > > >               the version to 0.10 we are breaking the
>> KafkaConsumer
>> > > > >               calls in our Storm spout. In storm's case we ask
>> users
>> > to
>> > > > >               create uber jar with all the required dependencies
>> and
>> > > > >               users can free to use which version of kafka they
>> can
>> > to
>> > > > >               be part of uber jar. If they use storm 1.0 release
>> > version
>> > > > >               of storm-kafka with kafka 0.10 than it will create
>> > issues
>> > > > >               without the patch.
>> > > > >              I am still not getting clear answer here. Whats
>> exactly
>> > the
>> > > > >              issue in having these methods with deprecated tag? we
>> > keep
>> > > > >              the interface as it is.
>> > > > >
>> > > > > Thanks,
>> > > > > Harsha
>> > > > >
>> > > > > On Thu, Apr 28, 2016, at 01:27 PM, Ismael Juma wrote:
>> > > > > > Hi Harsha,
>> > > > > >
>> > > > > > What is the aim of the PR, is it to fix binary compatibility,
>> > source
>> > > > > > compatibility or both? I think it only fixes source
>> compatibility,
>> > so I
>> > > > > > am
>> > > > > > interested in what testing has been done to ensure that this fix
>> > solves
>> > > > > > the
>> > > > > > Storm issue.
>> > > > > >
>> > > > > > Thanks,
>> > > > > > Ismael
>> > > > > >
>> > > > > > On Thu, Apr 28, 2016 at 12:58 PM, Harsha <ka...@harsha.io>
>> wrote:
>> > > > > >
>> > > > > > > Hi,
>> > > > > > >        We missed this vote earlier and realized thats its
>> > breaking
>> > > > the
>> > > > > > >        0.9.x client api compatibility.  I opened a JIRA here
>> > > > > > >        https://issues.apache.org/jira/browse/KAFKA-3633 .
>> Can we
>> > > > keep
>> > > > > > >        the old methods with deprecated tag in 0.10 release.
>> > > > > > >
>> > > > > > > Thanks,
>> > > > > > > Harsha
>> > > > > > >
>> > > > > > > On Fri, Mar 18, 2016, at 01:51 PM, Jason Gustafson wrote:
>> > > > > > > > Looks like the KIP has passed. The finally tally is +5 among
>> > > > > committers
>> > > > > > > > and
>> > > > > > > > +9 overall.
>> > > > > > > >
>> > > > > > > > Thanks to Pierre-Yves Ritschard for all of the hard work and
>> > > > > persistence
>> > > > > > > > with this!
>> > > > > > > >
>> > > > > > > > -Jason
>> > > > > > > >
>> > > > > > > > On Wed, Mar 16, 2016 at 9:01 PM, Ewen Cheslack-Postava
>> > > > > > > > <e...@confluent.io>
>> > > > > > > > wrote:
>> > > > > > > >
>> > > > > > > > > +1.
>> > > > > > > > >
>> > > > > > > > > Normally I'd be more of a stickler for compatibility, but
>> > this is
>> > > > > new,
>> > > > > > > I
>> > > > > > > > > think it's worth emphasizing that unstable actually means
>> > > > unstable
>> > > > > &
>> > > > > > > might
>> > > > > > > > > require recompile (and maybe even adapting code when we
>> > think the
>> > > > > > > change
>> > > > > > > > > warrants it), and I think the impact is relatively low
>> since
>> > > > those
>> > > > > > > adopting
>> > > > > > > > > the new consumer know that it's very new. Agreed with
>> > Guozhang
>> > > > that
>> > > > > > > better
>> > > > > > > > > documenting the annotations will help (and personally
>> > apologize
>> > > > for
>> > > > > > > that
>> > > > > > > > > since we hastily introduced the annotations to give
>> ourselves
>> > > > > wiggle
>> > > > > > > room
>> > > > > > > > > on Connect).
>> > > > > > > > >
>> > > > > > > > > -Ewen
>> > > > > > > > >
>> > > > > > > > > On Wed, Mar 16, 2016 at 5:17 PM, Joel Koshy <
>> > jjkosh...@gmail.com
>> > > > >
>> > > > > > > wrote:
>> > > > > > > > >
>> > > > > > > > > > +1
>> > > > > > > > > >
>> > > > > > > > > > On Tue, Mar 15, 2016 at 2:18 PM, Jason Gustafson <
>> > > > > ja...@confluent.io
>> > > > > > > >
>> > > > > > > > > > wrote:
>> > > > > > > > > >
>> > > > > > > > > > > I'd like to open the vote for KIP-45. We've discussed
>> > several
>> > > > > > > > > > alternatives
>> > > > > > > > > > > on the mailing list and in the KIP call, but this
>> vote is
>> > > > only
>> > > > > on
>> > > > > > > the
>> > > > > > > > > > > documented KIP:
>> > > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > >
>> > > > > > >
>> > > > >
>> > > >
>> >
>> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=61337336
>> .
>> > > > > > > > > > > This
>> > > > > > > > > > > change will not be compatible with 0.9, but it will
>> > provide a
>> > > > > > > cleaner
>> > > > > > > > > API
>> > > > > > > > > > > long term for users to work with. This is really the
>> last
>> > > > > chance to
>> > > > > > > > > make
>> > > > > > > > > > an
>> > > > > > > > > > > incompatible change like this with 0.10 shortly on the
>> > way,
>> > > > but
>> > > > > > > > > > compatible
>> > > > > > > > > > > options (such as method overloading) could be brought
>> up
>> > > > again
>> > > > > in
>> > > > > > > the
>> > > > > > > > > > > future if we find it's needed.
>> > > > > > > > > > >
>> > > > > > > > > > > Thanks,
>> > > > > > > > > > > Jason
>> > > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > >
>> > > > > > > > >
>> > > > > > > > >
>> > > > > > > > > --
>> > > > > > > > > Thanks,
>> > > > > > > > > Ewen
>> > > > > > > > >
>> > > > > > >
>> > > > >
>> > > >
>> > >
>> > >
>> > >
>> > > --
>> > > Grant Henke
>> > > Software Engineer | Cloudera
>> > > gr...@cloudera.com | twitter.com/gchenke | linkedin.com/in/granthenke
>> >
>>
>
>
>
> --
> Grant Henke
> Software Engineer | Cloudera
> gr...@cloudera.com | twitter.com/gchenke | linkedin.com/in/granthenke
>



-- 
Grant Henke
Software Engineer | Cloudera
gr...@cloudera.com | twitter.com/gchenke | linkedin.com/in/granthenke

Reply via email to