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
