Ismael, thanks for the review. On Fri, Apr 1, 2016 at 1:22 AM, Ismael Juma <ism...@juma.me.uk> wrote:
> A couple of questions: > > 1. The KIP says "Specific version may be deprecated through protocol > documentation but must still be supported (although it is fair to return an > error code if the specific API supports it).". It may be worth expanding > this a little more. For example, what does it mean to support the API? I > guess this means that the broker must not disconnect the client and the > broker must return a valid protocol response. Sure, will add to KIP. > Given that it says that it is > "fair" (I would probably replace "fair" with "valid") to return an error > code if the specific API supports it, it sounds like we are saying that we > don't have to maintain the semantic behaviour (i.e. we could _always_ > return an error for a deprecated API?). Is this true? > Yes. Typically we should support deprecated APIs, however in cases where an intermediate buggy version was backported, if the api has error field, it should be valid to use it to signal an error. Example of such backporting is provided on KIP, below is excerpt. Note that this KIP is not asking for any new error type. > 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). > > 2. ApiVersionQueryRequest seems a bit verbose, why not ApiVersionRequest? > Adopted. > > Ismael > -- Regards, Ashish
