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
