Re: Review request for KAFKA-3600

[Ashish Singh]

I am open to any forum for design discussions. I had run this by Jason and
gotten some feedback, four months back. I have addressed those feedbacks,
and I believe not much have changed since then in the design. I would love
to have more feedback.

   -

   do we want to modify the state machine?
   I think it is better to have an explicit state that tells client is
   fetching api versions, rather than merging it into one of the existing
   states, say CONNECTING state.
   -

   do we need to maintain version per connection?
   If we assume we are only dealing with homogeneous clusters, where all
   brokers are of same version, then probably not. This however is not always
   true and I could not think of a reason why keeping per connection version
   is an issue.
   -

   which layer is responsible for the API check?
   Keeping it in NetworkClient avoids us from adding same check at multiple
   places. Also, NetworkClient can perform the check if the owner wants it
   to perform the check. In current version, api version is checked only for
   producer and consumer clients, but not for inter-broker communications.

​

On Mon, Aug 15, 2016 at 9:24 AM, Gwen Shapira  wrote:

> In general, I believe it is good to have design discussions in the
> mailing list (do we want to modify the state machine? do we need to
> maintain version per connection? which layer is responsible for the
> API check)?
>
> PR works well with tactical issues (class / object / method level) but
> less so for design discussions.
>
> The discussion doesn't need to be with the entire list (offlist
> discussion with reviewers worked well in the past), but I noticed that
> having at least one reviewer (not necessarily a committer - Jason
> would be a fantastic reviewer here) who understands and agrees with
> the design improves the discussion a lot.
>
> Gwen
>
> On Mon, Aug 15, 2016 at 8:27 AM, Ashish Singh  wrote:
> > Hello Dana,
> >
> > Thanks for the feedback.
> >
> > Kafka-3600 is not intended to address backwards compatibility of clients.
> > Backwards compatibility of clients is a lengthier discussion and it will
> be
> > better to take it on a separate KIP. We initiated that discussion on
> > KIP-35's discuss thread and it was hard to reach a consensus there.
> > However, much has changed since then and there are at least couple of
> > non-java clients that support backwards compatibility and they do it
> > differently. I too believe that having a consensus on what backwards
> > compatibility in Kafka clients should mean and how to achieve it is very
> > important. However, it is not intended to be tackled in this particular
> > JIRA.
> >
> > As you mentioned, with KAFKA-3600 applications won't see no response and
> an
> > abruptly closed socket when they try to talk to an unsupported broker
> > version. Let me try list out concerns raised.
> >
> > 1. Is it worth the complexity just to add this check?
> > Most of the work in the patch is to enhance NetworkClient to check api
> > versions supported by brokers it has connections to. This information
> will
> > be required by clients to enable features, as done by librdkafka, or to
> > infer api version used by broker, IIUC as done by kafka-python.
> >
> > 2. Is a new connection state required?
> > The new connection state helps in identifying what is a client doing
> right
> > now. Having too many steps/ responsibilities in same step makes it hard
> to
> > understand what a client is doing in a particular step or to debug. But,
> we
> > can discuss more about this on PR.
> >
> > 3. Is network client required to maintain node api versions?
> > Absolutely not. I figured feature enablement would require this following
> > librdkafka's model. But I agree that as we have not yet decided on how to
> > implement backwards compatibility, we should not put in any building
> block
> > for a particular solution. I will be happy to take it out.
> >
> > Does above answers/ explanations address your concerns?
> >
> > On Fri, Aug 12, 2016 at 10:03 AM, Dana Powers 
> wrote:
> >
> >> I think one of the hard parts is that while we agree that a call to
> >> ApiVersions is required, I don't think there is agreement on how to
> >> use the response.
> >>
> >> In kafka-python, for example, to support backwards compatibility we
> >> set a single "api" configuration value during the client constructor.
> >> This value is either set by the user explicitly via configuration or,
> >> if unset by the user, inferred from the first bootstrap server that
> >> responds. In either case we are choosing 1 "api" to apply to all
> >> broker connections. We then use this configuration throughout the
> >> client code to choose different feature paths (Zookeeper v. Kafka
> >> offsets, for example; or whether to use group coordination apis v.
> >> assign all partitions locally on 'subscribe'; etc).
> >>
> >> We do not short-circuit requests that appear unsupported 

Re: Review request for KAFKA-3600

[Gwen Shapira]

In general, I believe it is good to have design discussions in the
mailing list (do we want to modify the state machine? do we need to
maintain version per connection? which layer is responsible for the
API check)?

PR works well with tactical issues (class / object / method level) but
less so for design discussions.

The discussion doesn't need to be with the entire list (offlist
discussion with reviewers worked well in the past), but I noticed that
having at least one reviewer (not necessarily a committer - Jason
would be a fantastic reviewer here) who understands and agrees with
the design improves the discussion a lot.

Gwen

On Mon, Aug 15, 2016 at 8:27 AM, Ashish Singh  wrote:
> Hello Dana,
>
> Thanks for the feedback.
>
> Kafka-3600 is not intended to address backwards compatibility of clients.
> Backwards compatibility of clients is a lengthier discussion and it will be
> better to take it on a separate KIP. We initiated that discussion on
> KIP-35's discuss thread and it was hard to reach a consensus there.
> However, much has changed since then and there are at least couple of
> non-java clients that support backwards compatibility and they do it
> differently. I too believe that having a consensus on what backwards
> compatibility in Kafka clients should mean and how to achieve it is very
> important. However, it is not intended to be tackled in this particular
> JIRA.
>
> As you mentioned, with KAFKA-3600 applications won't see no response and an
> abruptly closed socket when they try to talk to an unsupported broker
> version. Let me try list out concerns raised.
>
> 1. Is it worth the complexity just to add this check?
> Most of the work in the patch is to enhance NetworkClient to check api
> versions supported by brokers it has connections to. This information will
> be required by clients to enable features, as done by librdkafka, or to
> infer api version used by broker, IIUC as done by kafka-python.
>
> 2. Is a new connection state required?
> The new connection state helps in identifying what is a client doing right
> now. Having too many steps/ responsibilities in same step makes it hard to
> understand what a client is doing in a particular step or to debug. But, we
> can discuss more about this on PR.
>
> 3. Is network client required to maintain node api versions?
> Absolutely not. I figured feature enablement would require this following
> librdkafka's model. But I agree that as we have not yet decided on how to
> implement backwards compatibility, we should not put in any building block
> for a particular solution. I will be happy to take it out.
>
> Does above answers/ explanations address your concerns?
>
> On Fri, Aug 12, 2016 at 10:03 AM, Dana Powers  wrote:
>
>> I think one of the hard parts is that while we agree that a call to
>> ApiVersions is required, I don't think there is agreement on how to
>> use the response.
>>
>> In kafka-python, for example, to support backwards compatibility we
>> set a single "api" configuration value during the client constructor.
>> This value is either set by the user explicitly via configuration or,
>> if unset by the user, inferred from the first bootstrap server that
>> responds. In either case we are choosing 1 "api" to apply to all
>> broker connections. We then use this configuration throughout the
>> client code to choose different feature paths (Zookeeper v. Kafka
>> offsets, for example; or whether to use group coordination apis v.
>> assign all partitions locally on 'subscribe'; etc).
>>
>> We do not short-circuit requests that appear unsupported by a
>> particular broker based on this api configuration. We send all
>> requests normally. If the broker doesn't understand a request, we
>> expect it to fail normally. Of course, "normally" here means no
>> response and an abruptly closed socket, which I think could be
>> improved.
>>
>> So KAFKA-3600. At its core, this revision implements a check on each
>> connection guaranteeing support for a configured api vector. If any
>> connection does not support that vector, the client raises a
>> KafkaException in the response handler.
>>
>> I think this is definitely an improvement. Compatibility errors are
>> completely opaque right now and can be very difficult to diagnose for
>> users that do not have direct access to their kafka cluster / brokers,
>> and/or do not have a firm grasp of the internals of the kafka client
>> and kafka protocol. But I also don't think this error handling is core
>> to compatibility. While supporting backwards compatibility, we
>> continue to rely on standard error handling (indeed, we have no choice
>> since we support backwards compatibility to 0.8.0).
>>
>> So I think the revision is attempting to add a few things that are
>> assumed to be building blocks for a compatibility layer:
>> (1) ApiVersions request / response handler;
>> (2) a new connection state,
>> (3) a hashmap of nodeApiVersions {node_id -> 

Re: Review request for KAFKA-3600

[Ashish Singh]

Hello Dana,

Thanks for the feedback.

Kafka-3600 is not intended to address backwards compatibility of clients.
Backwards compatibility of clients is a lengthier discussion and it will be
better to take it on a separate KIP. We initiated that discussion on
KIP-35's discuss thread and it was hard to reach a consensus there.
However, much has changed since then and there are at least couple of
non-java clients that support backwards compatibility and they do it
differently. I too believe that having a consensus on what backwards
compatibility in Kafka clients should mean and how to achieve it is very
important. However, it is not intended to be tackled in this particular
JIRA.

As you mentioned, with KAFKA-3600 applications won't see no response and an
abruptly closed socket when they try to talk to an unsupported broker
version. Let me try list out concerns raised.

1. Is it worth the complexity just to add this check?
Most of the work in the patch is to enhance NetworkClient to check api
versions supported by brokers it has connections to. This information will
be required by clients to enable features, as done by librdkafka, or to
infer api version used by broker, IIUC as done by kafka-python.

2. Is a new connection state required?
The new connection state helps in identifying what is a client doing right
now. Having too many steps/ responsibilities in same step makes it hard to
understand what a client is doing in a particular step or to debug. But, we
can discuss more about this on PR.

3. Is network client required to maintain node api versions?
Absolutely not. I figured feature enablement would require this following
librdkafka's model. But I agree that as we have not yet decided on how to
implement backwards compatibility, we should not put in any building block
for a particular solution. I will be happy to take it out.

Does above answers/ explanations address your concerns?

On Fri, Aug 12, 2016 at 10:03 AM, Dana Powers  wrote:

> I think one of the hard parts is that while we agree that a call to
> ApiVersions is required, I don't think there is agreement on how to
> use the response.
>
> In kafka-python, for example, to support backwards compatibility we
> set a single "api" configuration value during the client constructor.
> This value is either set by the user explicitly via configuration or,
> if unset by the user, inferred from the first bootstrap server that
> responds. In either case we are choosing 1 "api" to apply to all
> broker connections. We then use this configuration throughout the
> client code to choose different feature paths (Zookeeper v. Kafka
> offsets, for example; or whether to use group coordination apis v.
> assign all partitions locally on 'subscribe'; etc).
>
> We do not short-circuit requests that appear unsupported by a
> particular broker based on this api configuration. We send all
> requests normally. If the broker doesn't understand a request, we
> expect it to fail normally. Of course, "normally" here means no
> response and an abruptly closed socket, which I think could be
> improved.
>
> So KAFKA-3600. At its core, this revision implements a check on each
> connection guaranteeing support for a configured api vector. If any
> connection does not support that vector, the client raises a
> KafkaException in the response handler.
>
> I think this is definitely an improvement. Compatibility errors are
> completely opaque right now and can be very difficult to diagnose for
> users that do not have direct access to their kafka cluster / brokers,
> and/or do not have a firm grasp of the internals of the kafka client
> and kafka protocol. But I also don't think this error handling is core
> to compatibility. While supporting backwards compatibility, we
> continue to rely on standard error handling (indeed, we have no choice
> since we support backwards compatibility to 0.8.0).
>
> So I think the revision is attempting to add a few things that are
> assumed to be building blocks for a compatibility layer:
> (1) ApiVersions request / response handler;
> (2) a new connection state,
> (3) a hashmap of nodeApiVersions {node_id -> [api_versions]} .
>
> I'm confident that #1 is necessary. Is a new connection state
> necessary? Is a full map of all nodes -> supported api version
> necessary? These aren't used in the kafka-python implementation, so
> from my perspective the answer is no. But maybe the java community has
> some interesting ideas for improvement where these are key
> ingredients. In any case, I think it would probably help substantially
> to get some agreement on where the goalposts are for compatibility to
> help push this forward.
>
> -Dana
>
> On Fri, Aug 12, 2016 at 7:37 AM, Ashish Singh  wrote:
> > Most of the work in the patch is to enhance NetworkClient to maintain api
> > versions supported by brokers it has connections to. When a broker
> > disconnects, its api versions info is removed and when it reconnects 

Re: Review request for KAFKA-3600

[Dana Powers]

I think one of the hard parts is that while we agree that a call to
ApiVersions is required, I don't think there is agreement on how to
use the response.

In kafka-python, for example, to support backwards compatibility we
set a single "api" configuration value during the client constructor.
This value is either set by the user explicitly via configuration or,
if unset by the user, inferred from the first bootstrap server that
responds. In either case we are choosing 1 "api" to apply to all
broker connections. We then use this configuration throughout the
client code to choose different feature paths (Zookeeper v. Kafka
offsets, for example; or whether to use group coordination apis v.
assign all partitions locally on 'subscribe'; etc).

We do not short-circuit requests that appear unsupported by a
particular broker based on this api configuration. We send all
requests normally. If the broker doesn't understand a request, we
expect it to fail normally. Of course, "normally" here means no
response and an abruptly closed socket, which I think could be
improved.

So KAFKA-3600. At its core, this revision implements a check on each
connection guaranteeing support for a configured api vector. If any
connection does not support that vector, the client raises a
KafkaException in the response handler.

I think this is definitely an improvement. Compatibility errors are
completely opaque right now and can be very difficult to diagnose for
users that do not have direct access to their kafka cluster / brokers,
and/or do not have a firm grasp of the internals of the kafka client
and kafka protocol. But I also don't think this error handling is core
to compatibility. While supporting backwards compatibility, we
continue to rely on standard error handling (indeed, we have no choice
since we support backwards compatibility to 0.8.0).

So I think the revision is attempting to add a few things that are
assumed to be building blocks for a compatibility layer:
(1) ApiVersions request / response handler;
(2) a new connection state,
(3) a hashmap of nodeApiVersions {node_id -> [api_versions]} .

I'm confident that #1 is necessary. Is a new connection state
necessary? Is a full map of all nodes -> supported api version
necessary? These aren't used in the kafka-python implementation, so
from my perspective the answer is no. But maybe the java community has
some interesting ideas for improvement where these are key
ingredients. In any case, I think it would probably help substantially
to get some agreement on where the goalposts are for compatibility to
help push this forward.

-Dana

On Fri, Aug 12, 2016 at 7:37 AM, Ashish Singh  wrote:
> Most of the work in the patch is to enhance NetworkClient to maintain api
> versions supported by brokers it has connections to. When a broker
> disconnects, its api versions info is removed and when it reconnects that
> info is fetched again. In short, with these changes Network Client at any
> given point knows what are the various brokers it is connected to and what
> are the various apis they support. This information is required to enable/
> select features based on brokers the client is talking to. I want to do the
> following in the order mentioned.
>
>- Enable clients to detect, and so inform applications, that they are
>not compatible with brokers they are trying to talk to, rather than getting
>connection dropped or similar not so sure response. This has been ready for
>some time, so we can definitely work to include in next release.
>- Use the api versions info from NetworkClient to enable/ disable
>feature sets, something similar to what librdkafka already has. This will
>need much broader testing.
>
> Note that both of the aforementioned steps have been discussed as part of
> KIP-35 discussions, but only first step was voted in. We will have to go
> through another KIP for second step.
>
> Any suggestions on how to make progress here will be helpful. Dana has left
> some comments and I will address them soon. However, I would appreciate if
> I can get a committer willing to help merging this in.
>
>
> On Thu, Aug 11, 2016 at 4:15 PM, Gwen Shapira  wrote:
>
>> I am 100% pro smart Java clients that support KIP-35 and can use it to
>> work with newer brokers. If this JIRA makes sense as a step in that
>> direction, I think its great and remove my objection.  I didn't see
>> anything that looked like a plan toward full forward-backward
>> compatibility, which is why I responded as I did...
>>
>> Verification is good, but it looked like there was much complexity
>> added toward very little benefits.
>>
>> On Thu, Aug 11, 2016 at 3:37 PM, Ashish Singh  wrote:
>> > Hey Gwen,
>> >
>> > I think this was more than a verification step, it was a building step
>> > towards a backwards compatible clients or for clients that can select
>> > feature based on brokers it is talking to. Are we now against the 

Re: Review request for KAFKA-3600

[Ashish Singh]

Most of the work in the patch is to enhance NetworkClient to maintain api
versions supported by brokers it has connections to. When a broker
disconnects, its api versions info is removed and when it reconnects that
info is fetched again. In short, with these changes Network Client at any
given point knows what are the various brokers it is connected to and what
are the various apis they support. This information is required to enable/
select features based on brokers the client is talking to. I want to do the
following in the order mentioned.

   - Enable clients to detect, and so inform applications, that they are
   not compatible with brokers they are trying to talk to, rather than getting
   connection dropped or similar not so sure response. This has been ready for
   some time, so we can definitely work to include in next release.
   - Use the api versions info from NetworkClient to enable/ disable
   feature sets, something similar to what librdkafka already has. This will
   need much broader testing.

Note that both of the aforementioned steps have been discussed as part of
KIP-35 discussions, but only first step was voted in. We will have to go
through another KIP for second step.

Any suggestions on how to make progress here will be helpful. Dana has left
some comments and I will address them soon. However, I would appreciate if
I can get a committer willing to help merging this in.
​

On Thu, Aug 11, 2016 at 4:15 PM, Gwen Shapira  wrote:

> I am 100% pro smart Java clients that support KIP-35 and can use it to
> work with newer brokers. If this JIRA makes sense as a step in that
> direction, I think its great and remove my objection.  I didn't see
> anything that looked like a plan toward full forward-backward
> compatibility, which is why I responded as I did...
>
> Verification is good, but it looked like there was much complexity
> added toward very little benefits.
>
> On Thu, Aug 11, 2016 at 3:37 PM, Ashish Singh  wrote:
> > Hey Gwen,
> >
> > I think this was more than a verification step, it was a building step
> > towards a backwards compatible clients or for clients that can select
> > feature based on brokers it is talking to. Are we now against the idea of
> > having smarter clients? This adds complexity to enable clients to inform
> > applications of incompatible broker versions, I think that is some value
> > add. In future, the api versions info can be used to take smarter
> decisions.
> >
> > On Wed, Aug 10, 2016 at 11:04 PM, Gwen Shapira 
> wrote:
> >
> >> I hate doing this, because Ashish has really been good about following
> >> up on the PR, but I'm questioning the usefulness of this patch.
> >>
> >> It adds non-trivial complexity to the client... with not much return
> >> on the investment, as far as I can see?
> >> When I first suggested it, it was before KIP-35 was merged and
> >> released and the intent was to validate KIP-35 (since I have low
> >> opinion of protocols that aren't used). Since then KIP-35 was already
> >> released, the followup turned more complex than we expected, I think.
> >> And I'm wondering if it is worth it.
> >>
> >> The work and followup from Ashish is still super appreciated, but I
> >> think we need more than appreciation - adding complexity to already
> >> complex clients need to have functional justification...
> >>
> >> Anyway, I was out of the loop for ages, so feel free to yell at me for
> >> missing the obvious.
> >>
> >> Gwen
> >>
> >> On Tue, Aug 9, 2016 at 8:47 AM, Ashish Singh 
> wrote:
> >> > Provided wrong link to PR, here is the PR
> >> >  for KAFKA-3600.
> >> >
> >> > On Tue, Aug 9, 2016 at 9:45 AM, Ashish Singh 
> >> wrote:
> >> >
> >> >> Hey Guys,
> >> >>
> >> >> KAFKA-3600  was
> part
> >> of
> >> >> KIP-35's proposal. KAFKA-3307
> >> >> ,
> >> >> adding ApiVersionsRequest/Response, was committed to 0.10.0.0, but
> >> >> KAFKA-3600, enhancing java clients, is still under review. Here is
> the
> >> PR
> >> >> 
> >> >>
> >> >> I have addressed all review comments and have been waiting for
> further
> >> >> reviews/ this to go in for quite some time. I will really appreciate
> if
> >> a
> >> >> committer can help with making progress on this.
> >> >>
> >> >> --
> >> >>
> >> >> Regards,
> >> >> Ashish
> >> >>
> >> >
> >> >
> >> >
> >> > --
> >> >
> >> > Regards,
> >> > Ashish
> >>
> >>
> >>
> >> --
> >> Gwen Shapira
> >> Product Manager | Confluent
> >> 650.450.2760 | @gwenshap
> >> Follow us: Twitter | blog
> >>
> >
> >
> >
> > --
> >
> > Regards,
> > Ashish
>
>
>
> --
> Gwen Shapira
> Product Manager | Confluent
> 650.450.2760 | @gwenshap
> Follow us: Twitter | blog
>



-- 

Regards,
Ashish

Re: Review request for KAFKA-3600

[Gwen Shapira]

I am 100% pro smart Java clients that support KIP-35 and can use it to
work with newer brokers. If this JIRA makes sense as a step in that
direction, I think its great and remove my objection.  I didn't see
anything that looked like a plan toward full forward-backward
compatibility, which is why I responded as I did...

Verification is good, but it looked like there was much complexity
added toward very little benefits.

On Thu, Aug 11, 2016 at 3:37 PM, Ashish Singh  wrote:
> Hey Gwen,
>
> I think this was more than a verification step, it was a building step
> towards a backwards compatible clients or for clients that can select
> feature based on brokers it is talking to. Are we now against the idea of
> having smarter clients? This adds complexity to enable clients to inform
> applications of incompatible broker versions, I think that is some value
> add. In future, the api versions info can be used to take smarter decisions.
>
> On Wed, Aug 10, 2016 at 11:04 PM, Gwen Shapira  wrote:
>
>> I hate doing this, because Ashish has really been good about following
>> up on the PR, but I'm questioning the usefulness of this patch.
>>
>> It adds non-trivial complexity to the client... with not much return
>> on the investment, as far as I can see?
>> When I first suggested it, it was before KIP-35 was merged and
>> released and the intent was to validate KIP-35 (since I have low
>> opinion of protocols that aren't used). Since then KIP-35 was already
>> released, the followup turned more complex than we expected, I think.
>> And I'm wondering if it is worth it.
>>
>> The work and followup from Ashish is still super appreciated, but I
>> think we need more than appreciation - adding complexity to already
>> complex clients need to have functional justification...
>>
>> Anyway, I was out of the loop for ages, so feel free to yell at me for
>> missing the obvious.
>>
>> Gwen
>>
>> On Tue, Aug 9, 2016 at 8:47 AM, Ashish Singh  wrote:
>> > Provided wrong link to PR, here is the PR
>> >  for KAFKA-3600.
>> >
>> > On Tue, Aug 9, 2016 at 9:45 AM, Ashish Singh 
>> wrote:
>> >
>> >> Hey Guys,
>> >>
>> >> KAFKA-3600  was part
>> of
>> >> KIP-35's proposal. KAFKA-3307
>> >> ,
>> >> adding ApiVersionsRequest/Response, was committed to 0.10.0.0, but
>> >> KAFKA-3600, enhancing java clients, is still under review. Here is the
>> PR
>> >> 
>> >>
>> >> I have addressed all review comments and have been waiting for further
>> >> reviews/ this to go in for quite some time. I will really appreciate if
>> a
>> >> committer can help with making progress on this.
>> >>
>> >> --
>> >>
>> >> Regards,
>> >> Ashish
>> >>
>> >
>> >
>> >
>> > --
>> >
>> > Regards,
>> > Ashish
>>
>>
>>
>> --
>> Gwen Shapira
>> Product Manager | Confluent
>> 650.450.2760 | @gwenshap
>> Follow us: Twitter | blog
>>
>
>
>
> --
>
> Regards,
> Ashish



-- 
Gwen Shapira
Product Manager | Confluent
650.450.2760 | @gwenshap
Follow us: Twitter | blog

Re: Review request for KAFKA-3600

[Ashish Singh]

Hey Gwen,

I think this was more than a verification step, it was a building step
towards a backwards compatible clients or for clients that can select
feature based on brokers it is talking to. Are we now against the idea of
having smarter clients? This adds complexity to enable clients to inform
applications of incompatible broker versions, I think that is some value
add. In future, the api versions info can be used to take smarter decisions.

On Wed, Aug 10, 2016 at 11:04 PM, Gwen Shapira  wrote:

> I hate doing this, because Ashish has really been good about following
> up on the PR, but I'm questioning the usefulness of this patch.
>
> It adds non-trivial complexity to the client... with not much return
> on the investment, as far as I can see?
> When I first suggested it, it was before KIP-35 was merged and
> released and the intent was to validate KIP-35 (since I have low
> opinion of protocols that aren't used). Since then KIP-35 was already
> released, the followup turned more complex than we expected, I think.
> And I'm wondering if it is worth it.
>
> The work and followup from Ashish is still super appreciated, but I
> think we need more than appreciation - adding complexity to already
> complex clients need to have functional justification...
>
> Anyway, I was out of the loop for ages, so feel free to yell at me for
> missing the obvious.
>
> Gwen
>
> On Tue, Aug 9, 2016 at 8:47 AM, Ashish Singh  wrote:
> > Provided wrong link to PR, here is the PR
> >  for KAFKA-3600.
> >
> > On Tue, Aug 9, 2016 at 9:45 AM, Ashish Singh 
> wrote:
> >
> >> Hey Guys,
> >>
> >> KAFKA-3600  was part
> of
> >> KIP-35's proposal. KAFKA-3307
> >> ,
> >> adding ApiVersionsRequest/Response, was committed to 0.10.0.0, but
> >> KAFKA-3600, enhancing java clients, is still under review. Here is the
> PR
> >> 
> >>
> >> I have addressed all review comments and have been waiting for further
> >> reviews/ this to go in for quite some time. I will really appreciate if
> a
> >> committer can help with making progress on this.
> >>
> >> --
> >>
> >> Regards,
> >> Ashish
> >>
> >
> >
> >
> > --
> >
> > Regards,
> > Ashish
>
>
>
> --
> Gwen Shapira
> Product Manager | Confluent
> 650.450.2760 | @gwenshap
> Follow us: Twitter | blog
>



-- 

Regards,
Ashish

Re: Review request for KAFKA-3600

[Gwen Shapira]

I hate doing this, because Ashish has really been good about following
up on the PR, but I'm questioning the usefulness of this patch.

It adds non-trivial complexity to the client... with not much return
on the investment, as far as I can see?
When I first suggested it, it was before KIP-35 was merged and
released and the intent was to validate KIP-35 (since I have low
opinion of protocols that aren't used). Since then KIP-35 was already
released, the followup turned more complex than we expected, I think.
And I'm wondering if it is worth it.

The work and followup from Ashish is still super appreciated, but I
think we need more than appreciation - adding complexity to already
complex clients need to have functional justification...

Anyway, I was out of the loop for ages, so feel free to yell at me for
missing the obvious.

Gwen

On Tue, Aug 9, 2016 at 8:47 AM, Ashish Singh  wrote:
> Provided wrong link to PR, here is the PR
>  for KAFKA-3600.
>
> On Tue, Aug 9, 2016 at 9:45 AM, Ashish Singh  wrote:
>
>> Hey Guys,
>>
>> KAFKA-3600  was part of
>> KIP-35's proposal. KAFKA-3307
>> ,
>> adding ApiVersionsRequest/Response, was committed to 0.10.0.0, but
>> KAFKA-3600, enhancing java clients, is still under review. Here is the PR
>> 
>>
>> I have addressed all review comments and have been waiting for further
>> reviews/ this to go in for quite some time. I will really appreciate if a
>> committer can help with making progress on this.
>>
>> --
>>
>> Regards,
>> Ashish
>>
>
>
>
> --
>
> Regards,
> Ashish



-- 
Gwen Shapira
Product Manager | Confluent
650.450.2760 | @gwenshap
Follow us: Twitter | blog

Re: Review request for KAFKA-3600

[Ashish Singh]

Provided wrong link to PR, here is the PR
 for KAFKA-3600.

On Tue, Aug 9, 2016 at 9:45 AM, Ashish Singh  wrote:

> Hey Guys,
>
> KAFKA-3600  was part of
> KIP-35's proposal. KAFKA-3307
> ,
> adding ApiVersionsRequest/Response, was committed to 0.10.0.0, but
> KAFKA-3600, enhancing java clients, is still under review. Here is the PR
> 
>
> I have addressed all review comments and have been waiting for further
> reviews/ this to go in for quite some time. I will really appreciate if a
> committer can help with making progress on this.
>
> --
>
> Regards,
> Ashish
>



-- 

Regards,
Ashish

Review request for KAFKA-3600

[Ashish Singh]

Hey Guys,

KAFKA-3600  was part of
KIP-35's proposal. KAFKA-3307
,
adding ApiVersionsRequest/Response, was committed to 0.10.0.0, but
KAFKA-3600, enhancing java clients, is still under review. Here is the PR


I have addressed all review comments and have been waiting for further
reviews/ this to go in for quite some time. I will really appreciate if a
committer can help with making progress on this.

-- 

Regards,
Ashish