+1 for option 3 (unfortunately all options have pros and cons, but I think option 3 does a better job with regards to trade-offs).
Ismael On Mon, May 9, 2016 at 7:56 PM, Gwen Shapira <g...@confluent.io> wrote: > We need to conclude this discussion in order to unblock the release. > > If I understand correctly, we have 3 options on the table: > > 1. Add the missing APIs as "deprecated". > Pros: Conceptually, the right way to get rid of an API without breaking > Storm > Cons: Users of the new API will get ugly warnings, and they are > actually doing the right thing. This will encourage bad usage of our > APIs. > > 2. Add the missing APIs and sticking with them forever. > Pros: Avoid breaking Storm and avoid ugly warnings > Cons: Uglier and more confusing APIs for Consumer for eternity. We > marked the API as @unstable specifically to avoid this scenario. > > 3. Leave things as is. Storm (and maybe few others) will need to do a > bit of reflection to support the new consumer in both 0.9.0 and 0.10.0 > branches. > Pros: Clean APIs for our Consumer, clear meaning for "@unstable" in > the community. > Cons: Some extra work for Storm. > > With Apache Kafka's interests in mind, I think #3 is the right way > forward. Anything else is worse for Kafka users. > > I'd love to hear from the rest of the members :) > > Gwen > > On Sun, May 8, 2016 at 1:56 AM, Ismael Juma <ism...@juma.me.uk> wrote: > > Hi Harsha, > > > > See inline. > > > > On Sun, May 8, 2016 at 3:02 AM, Harsha <ka...@harsha.io> wrote: > > > >> Ismael, > >> Do we need to add old assign and subscribe that accepts List. > > > > > > Yes, to maintain binary compatibility. If you compile your jar with > > kafka-clients 0.9.0.1 and then run it with kafka-clients 0.10.0.0 you > will > > get an error like: > > > > ~/t/binary-compat-test ❯❯❯ java -cp > > > src:lib/kafka-clients-0.10.0.0.jar:lib/slf4j-api-1.7.21.jar:lib/slf4j-log4j12-1.7.21.jar:lib/log4j-1.2.17.jar > > test.BinaryCompat > > Exception in thread "main" java.lang.NoSuchMethodError: > > > org.apache.kafka.clients.consumer.KafkaConsumer.subscribe(Ljava/util/List;)V > > at test.BinaryCompat.main(BinaryCompat.java:21) > > > > That is a real error from a simple test I did. This is why I asked in my > > original message if you had tested that your proposed PR fixed your issue > > completely (I don't think it does). > > > > It will get implicitly cast to collection with the new methods. > >> > > > > That is only the case if the code is recompiled with 0.10.0.0 (i.e. > source > > compatibility). > > > > Hope this makes things clearer. > > > > Ismael > > > > P.S. Source code for the simple test I did: > > https://gist.github.com/ijuma/dfec36382779b5022989b4380af99b37 > > > > > >> The only problem comes from the methods that accepts varargs. > >> > >> -Harsha > >> > >> On Sat, May 7, 2016, at 05:53 PM, Mark Grover wrote: > >> > Thanks Ismael, I agree with you, it makes sense to leave things the > way > >> > they are in Kafka 0.10. > >> > > >> > On Fri, May 6, 2016 at 5:27 PM, Ismael Juma <ism...@juma.me.uk> > wrote: > >> > > >> > > Hi Mark, > >> > > > >> > > Thanks for the email. First of all, I'd like to mention that the > >> `Unstable` > >> > > annotation has been removed from the new Java consumer in 0.10, so > you > >> can > >> > > expect compatibility from now on. We definitely understand that > >> > > compatibility is important for widespread adoption. > >> > > > >> > > The current PR for KAFKA-3633 adds deprecated and overloaded methods > >> for > >> > > `seekToBeginning`, `seekToEnd`, `pause` and `resume` each taking a > >> varargs > >> > > parameter for backwards compatibility. If these methods solved the > >> binary > >> > > compatibility issue, I'd be supportive of adding them. > >> > > > >> > > However, as I pointed out in my original message (and Jason > elaborated > >> > > subsequently), something would also have to be done about `assign` > and > >> > > `subscribe` in order to maintain binary compatibility between 0.9 > and > >> 0.10. > >> > > And a good solution for these methods is elusive. > >> > > > >> > > If we add deprecated and overloaded methods that take a `List` > >> parameter, > >> > > then every existing user of the new consumer will be exposed to a > >> > > deprecated warning (or error if they have a warning as error build > >> policy) > >> > > because everyone uses `subscribe`. Avoiding the warning involves > using > >> > > `Set` instead of `List`, which is a bit weird and unintuitive > >> (although we > >> > > could document it). > >> > > > >> > > We could add the overloaded methods without deprecating them. In > this > >> case, > >> > > we would be stuck with two methods for the same thing forever (for > both > >> > > `subscribe` and `assign`). This makes the API more confusing and > >> overloads > >> > > mean that type inference from lambdas would be less effective (if at > >> all > >> > > effective). > >> > > > >> > > Or we could leave things as they are. The `subscribe` and `assign` > >> changes > >> > > are source compatible so no source changes are needed by the common > >> user > >> > > who just compiles against a particular version of the Kafka clients > >> > > library. It's also important to note that kafka-clients 0.9 works > fine > >> with > >> > > 0.10 brokers. Supporting both 0.9 and 0.10 clients from the same JAR > >> will > >> > > be a bit annoying, but the ugly shim code for that is > straightforward > >> to > >> > > write for advanced users that need this. > >> > > > >> > > I should make it clear that this is my position, other committers > may > >> feel > >> > > differently. > >> > > > >> > > Ismael > >> > > > >> > > On Sat, May 7, 2016 at 12:38 AM, Mark Grover <m...@apache.org> > wrote: > >> > > > >> > > > I understand and empathize with both sides of the story here. I > spend > >> > > some > >> > > > of my time on Spark and Kafka integration and I have cc'ed Cody > who's > >> > > been > >> > > > working on new Kafka consumer API with Spark Streaming. > >> > > > Spark hasn't merged the new Kafka consumer API integration, the PR > >> is up > >> > > > and we, as a community, are deliberating > >> > > > < > >> > > > > >> > > > >> > https://issues.apache.org/jira/browse/SPARK-12177?focusedCommentId=15274910&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-15274910 > >> > > > > > >> > > > if now is the right time to put this in, given the flux in the > API, > >> the > >> > > > lack of delegation tokens support, etc. > >> > > > > >> > > > The proposed Spark integration with Kafka's new API relies on > >> > > > KafkaConsumer::pause() and KafkaConsumer::seekToEnd() and those > >> methods > >> > > > break compatibility between 0.9 and 0.10 RC4 (since KAFKA-3633 > >> > > > <https://issues.apache.org/jira/browse/KAFKA-3633> remains > >> unresolved). > >> > > > > >> > > > What this means is that if Spark supports both 0.9 and 0.10, we > are > >> going > >> > > > to have some complexity to compile against 0.9 and 0.10. So, Spark > >> > > > community, Cody and myself are all leaning towards not putting > Kafka > >> 0.9 > >> > > > support in, and only supporting starting Kafka 0.10 (or, may be > even > >> > > later) > >> > > > depending on the compatibility situation. The point I am trying to > >> make > >> > > is > >> > > > that the new consumer API doesn't add much value for us just yet > and > >> > > > breaking compatibility doesn't help in encouraging us to add > support > >> for > >> > > it > >> > > > in Spark. > >> > > > > >> > > > As far as this particular topic (KAFKA-3633) goes, I don't have a > >> strong > >> > > > vote, since Spark isn't likely to support 0.9's new kafka consumer > >> API > >> > > > anyways. However, I'd state the obvious that compatibility is > >> important > >> > > if > >> > > > you'd like to encourage us to adopt the new API. > >> > > > > >> > > > Mark > >> > > > > >> > > > On Mon, May 2, 2016 at 10:45 AM, Guozhang Wang < > wangg...@gmail.com> > >> > > wrote: > >> > > > > >> > > > > Just my two cents here: > >> > > > > > >> > > > > I agree with Ewen and Grant on the indication of the "unstable" > >> > > > annotations > >> > > > > of being possible for backward incompatible. That means, users > can > >> > > make a > >> > > > > call themselves of whether to start trying out the new APIs / > >> libraries > >> > > > > with the risk or changing code when it changes in a later > release > >> or > >> > > just > >> > > > > wait for it to be stable. Personally I don't think it would > result > >> in > >> > > no > >> > > > > one ever going to try out "unstable" new APIs, even in their > >> production > >> > > > > usages (for example at LI we used the LiKafkaClient to wrap the > >> apache > >> > > > > kafka clients and one motivation is to abstract any API backward > >> > > > > incompatibility); for cases like the Storm integration > scenarios, > >> yes > >> > > it > >> > > > > may become a considering blocker for incorporating "unstable" > APIs, > >> > > > again I > >> > > > > think it is just a decision we will educate users to make > >> themselves. > >> > > > > > >> > > > > > >> > > > > Guozhang > >> > > > > > >> > > > > > >> > > > > On Sat, Apr 30, 2016 at 8:13 AM, Harsha <ka...@harsha.io> > wrote: > >> > > > > > >> > > > > > Hi Jason, > >> > > > > > Yes I am in favor removing them 0.11 and it > >> definitely > >> > > > gives > >> > > > > > everyone one major version to update their > clients to > >> > > > remove > >> > > > > > the deprecated commands. > >> > > > > > > >> > > > > > Thanks, > >> > > > > > Harsha > >> > > > > > > >> > > > > > On Fri, Apr 29, 2016, at 11:02 PM, Ewen Cheslack-Postava > wrote: > >> > > > > > > I agree with Grant that we really need to indicate to > >> consumers of > >> > > > APIs > >> > > > > > > that when we mark it as unstable, it *really* means > unstable. > >> This > >> > > > is a > >> > > > > > > more general problem of needing to define our APIs and > >> stability -- > >> > > > but > >> > > > > > > I's > >> > > > > > > say that while we probably were too hasty in adding APIs, it > >> was > >> > > > > probably > >> > > > > > > better to add *some* indication of stability and support > than > >> just > >> > > > add > >> > > > > > > APIs > >> > > > > > > with not promises. > >> > > > > > > > >> > > > > > > On the other hand, since I helped introduce the Unstable > >> > > annotation, > >> > > > > even > >> > > > > > > then it wasn't entirely clear what it meant, and I am a firm > >> > > believer > >> > > > > in > >> > > > > > > attempting to provide *some* migration period for > incompatible > >> > > > > changes, I > >> > > > > > > would be more than happy to adapt the public API to provide > >> > > backwards > >> > > > > > > compatibility for those APIs for *at least* one release. > >> > > > > > > > >> > > > > > > Is there a strong reason for not doing this that isn't > >> > > incompatible? > >> > > > > > > > >> > > > > > > And shouldn't we try to be as helpful to consumers of our > *new* > >> > > APIs > >> > > > as > >> > > > > > > possible -- we want them to adopt new APIs! If there's a > small > >> > > amount > >> > > > > of > >> > > > > > > effort on our part that keeps things compatible, at least > over > >> the > >> > > > > course > >> > > > > > > of a major release, it encourages downstream projects to try > >> our > >> > > APIs > >> > > > > > > earlier, and that's a good thing. It won't always be > perfect; > >> > > > sometimes > >> > > > > > > we'll need to break major new features in a minor release; > but > >> in > >> > > > > > > general, > >> > > > > > > won't it be better? > >> > > > > > > > >> > > > > > > We should be very clear that we are going to remove these > APIs > >> with > >> > > > the > >> > > > > > > 0.11 release, which should hopefully make it clear what > Storm > >> can > >> > > > > expect > >> > > > > > > from us in terms of compatibility (noting, of course, that > we > >> make > >> > > no > >> > > > > > > real > >> > > > > > > promises currently about how long 0.10.x releases will be > >> made! we > >> > > > > > > already > >> > > > > > > make few guarantees about long term support). > >> > > > > > > > >> > > > > > > I know it would be ideal if all "external" stakeholders > could > >> get > >> > > > their > >> > > > > > > vote in with the KIP, but it's probably unrealistic to > expect > >> that > >> > > to > >> > > > > > > happen any time soon -- not everyone will see developments > in > >> the > >> > > > Kafka > >> > > > > > > project. I think we should give *a bit* of flexibility, > >> especially > >> > > > for > >> > > > > > > stuff we were all on the fence about, when these types of > >> issues > >> > > come > >> > > > > up. > >> > > > > > > > >> > > > > > > Everyone seemed to be on the fence previously. Is there a > good > >> > > reason > >> > > > > not > >> > > > > > > to adopt the suggested changes, that cost of a bit of > >> compatibility > >> > > > > pain? > >> > > > > > > > >> > > > > > > -Ewen > >> > > > > > > > >> > > > > > > > >> > > > > > > On Fri, Apr 29, 2016 at 7:58 PM, Jason Gustafson < > >> > > ja...@confluent.io > >> > > > > > >> > > > > > > wrote: > >> > > > > > > > >> > > > > > > > Hey Harsha, > >> > > > > > > > > >> > > > > > > > Just to clarify, are you ok with removing the methods in a > >> later > >> > > > > > release > >> > > > > > > > (say 0.11)? As I mentioned above, the only weird ones are > >> > > > subscribe() > >> > > > > > and > >> > > > > > > > assign(), which will have a deprecated version which > accepts > >> > > List. > >> > > > > > Users > >> > > > > > > > will have to change their code to use another collection > >> type or > >> > > a > >> > > > > > typecast > >> > > > > > > > to avoid deprecation warnings. That's annoying, but maybe > >> better > >> > > > than > >> > > > > > > > breaking compatibility. Does it make sense to update the > KIP > >> with > >> > > > > your > >> > > > > > > > proposal and request a new vote? > >> > > > > > > > > >> > > > > > > > Thanks, > >> > > > > > > > Jason > >> > > > > > > > > >> > > > > > > > On Fri, Apr 29, 2016 at 4:25 PM, Harsha <ka...@harsha.io> > >> wrote: > >> > > > > > > > > >> > > > > > > > > Grant, > >> > > > > > > > > I am sure this is discussed and voted. I've > seen > >> the > >> > > > > > > > > discussion. Given that there is an opportunity > to > >> make > >> > > > it > >> > > > > > less > >> > > > > > > > > painful for the users who shipped consumers > using > >> the > >> > > > > 0.9.x > >> > > > > > we > >> > > > > > > > > should consider that. > >> > > > > > > > > ". 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." > >> > > > > > > > > Yes we can tell everyone thats because we marked api > >> unstable > >> > > we > >> > > > > > gonna > >> > > > > > > > > break it in future release and not even consider make it > >> > > > > compatible. > >> > > > > > > > > With this approach I am sure no one would be interested > in > >> > > > writing > >> > > > > or > >> > > > > > > > > using any of the api's until they are stable and thats > not > >> way > >> > > to > >> > > > > vet > >> > > > > > > > > new apis. > >> > > > > > > > > > >> > > > > > > > > -Harsha > >> > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > On Fri, Apr 29, 2016, at 10:39 AM, Grant Henke wrote: > >> > > > > > > > > > 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 > >> > > > > > > > > > >> > > > > > > > > >> > > > > > > > >> > > > > > > > >> > > > > > > > >> > > > > > > -- > >> > > > > > > Thanks, > >> > > > > > > Ewen > >> > > > > > > >> > > > > > >> > > > > > >> > > > > > >> > > > > -- > >> > > > > -- Guozhang > >> > > > > > >> > > > > >> > > > >> > >
