+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 > > >
