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
