Good point Rajini, I will clarify that. Thanks, Magnus
2016-04-22 12:35 GMT-07:00 Rajini Sivaram <rajinisiva...@googlemail.com>: > +1 (non-binding) > > One minor comment: > > "11: The broker returns its full list of supported ApiKeys and versions > regardless of current authentication state (e.g., before SASL > authentication). If this is considered to leak information SSL can be used > for early authentication." > > > It may be better to explicitly state that ApiVersionRequest returns ApiKeys > and versions before SASL authentication (not regardless of current > authentication state) since you cannot send the request before SSL client > authentication, and you cannot send the request during SASL authentication. > > > On Fri, Apr 22, 2016 at 7:47 PM, Ismael Juma <ism...@juma.me.uk> wrote: > > > +1 (non-binding) from me, assuming that the changes suggested by Jun > below > > are included. > > > > Some minor comments. > > > > "5. Clients are recommended to use latest common supported API version." > > > > Maybe this would be clearer as: "Clients are recommended to use latest > > version supported by the broker and itself." > > > > "6. Deprecation of a protocol version will be done by marking a protocol > > version as deprecated in protocol documentation. Documentation shall also > > be used to indicate a protocol version that must not be used, or for any > > such information. For instance, say 0.9.0 had protocol versions [0] for > api > > key 1. On trunk, version 1 of the api key was added. Users running off > > trunk started using version 1 of the api and found out a major bug. To > > rectify that version 2 of the api is added to trunk. For some reason, it > is > > now deemed important to have version 2 of the api in 0.9.1 as well. To do > > so, version 1 and version 2 both of the api will be backported to the > 0.9.1 > > branch. 0.9.1 broker will return 0 as min supported version for the api > and > > 2 for the max supported version for the api. However, the version 1 > should > > be clearly marked as deprecated on its documentation. It will be client's > > responsibility to make sure they are not using any such deprecated > version > > to the best knowledge of the client at the time of development (or > > alternatively by configuration)." > > > > I still think that the mention of both 0.9.0 and 0.9.1 is confusing and > > it's not clear if we need it for this example. If we do need it, then we > > should define the state of 0.9.1 compared to 0.9.0 before we decide to > > backport APIs. > > > > "11. The broker returns its full list of supported ApiKeys and versions > > regardless of current authentication state (e.g., before SASL > > authentication). If this is considered to leak information SSL can be > used > > for early authentication." > > > > I think you mean "SSL with client authentication". > > > > Thanks, > > Ismael > > > > On Fri, Apr 22, 2016 at 11:31 AM, Jun Rao <j...@confluent.io> wrote: > > > > > Ashish, > > > > > > Just a couple of clarifications. > > > > > > 1. In ApiVersionRequest, we should get rid of ApiKeys since the request > > has > > > an empty body, right? > > > > > > 2. In ApiVersionResponse, we should list ErrorCode before ApiVersions, > > > right? > > > > > > Thanks, > > > > > > Jun > > > > > > On Thu, Apr 21, 2016 at 4:48 PM, Ashish Singh <asi...@cloudera.com> > > wrote: > > > > > > > Hey Guys, > > > > > > > > I would like to re-initiate the voting process for *KIP-35: Retrieve > > > > protocol version*. > > > > > > > > KIP-35 can be accessed here > > > > < > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-35+-+Retrieving+protocol+version > > > > >. > > > > Following are a couple of related PRs. > > > > > > > > 1. KAFKA-3307: Add ApiVersion request/response and server side > > > handling. > > > > <https://github.com/apache/kafka/pull/986> > > > > 2. KAFKA-3600: Enhance java clients to use ApiVersion Req/Resp to > > > check > > > > if the broker they are talking to supports required api versions. > > > > <https://github.com/apache/kafka/pull/1251> > > > > > > > > The vote will run for 72 hours and I would like to start it with my > +1 > > > > (non-binding). > > > > > > > > -- > > > > > > > > Regards, > > > > Ashish > > > > > > > > > > > > > -- > Regards, > > Rajini >
