Additionally feel free to review the PR (#1095 <https://github.com/apache/kafka/pull/1095>) to see what the current proposal/implementation looks like. Right now its using null to signify no topics. That can be changed based on the discussion here.
On Wed, Mar 30, 2016 at 2:34 PM, Grant Henke <ghe...@cloudera.com> wrote: > Thank you for the input everyone! I will try to address some of it below. > I will stick to the metadata related discussion first and we can go from > there. > > I would like to stay away from language/client style discussion to start. > Instead I think it would be useful to focus on reviewing the wire protocol > suggested. Languages have many ways of handling null, but this can all be > handled by the client API to be user friendly/clear. > > I think right now the most critical question, is if null vs empty list is > clear enough for protocol usage. Or is a boolean better and why? > > > MetadataRequest v1: long-term / conceptually, I think a "null" topic list >> aligns better with fetching all topics. Empty list aligns better with >> fetching no topics. I recognize this means that empty list behaves >> differently in v0 versus v1. But hey, what are protocol versions good for >> if not changing behavior... :) API design comment. take it or leave it. > > > I do agree that if we are making a change to use nulls, empty list = no > topics and null = all makes more sense. But it also makes the behavior > transition of the protocol a bit more confusing. Since the existing > behavior changes, not just the new behavior. > > > In the wire protocol, we would still use a size of -1 like we normally do >> for nullable strings and nullable bytes. So, it's still a bit magical, but >> efficient and associated with the right field (which avoids some invalid >> states that are possible if we use two fields). In other words, each >> implementation of the protocol is responsible for figuring out an >> idiomatic >> and hopefully safe way to represent the absence of a value. > > > Agreed. Thank you for summarizing. > > > Process comment: Since KIP-4 is voted and signed. Perhaps a small KIP >> detailing the suggested Metadata API changes so it will be easy to refer >> to >> what we are discussing? >> > > I would prefer not to break KIP-4 out into more KIPs, since the KIP > encompasses the high level goal (it the sum of the parts that is the > functionality/goal). That said, I do like breaking it up to get things in > and make progress. Could we just hold multiple votes for various pieces? In > this case we would be voting for the "KIP-4 Metadata API Changes". > > > On Wed, Mar 30, 2016 at 1:02 PM, Ismael Juma <ism...@juma.me.uk> wrote: > >> We can add an internal class until then (it's pretty trivial) since the >> request classes are internal. >> >> Ismael >> >> On Wed, Mar 30, 2016 at 7:00 PM, Gwen Shapira <g...@confluent.io> wrote: >> >> > I like it, but we are not on Java8 yet, and I don't think we want to >> block >> > on that :) >> > >> > On Wed, Mar 30, 2016 at 10:53 AM, Ismael Juma <ism...@juma.me.uk> >> wrote: >> > >> > > On Wed, Mar 30, 2016 at 6:21 PM, Gwen Shapira <g...@confluent.io> >> wrote: >> > > >> > > > Ismael, can you detail how the Optional approach would work in the >> wire >> > > > protocol? It sounds good, but I'm unclear on what this would look >> like >> > on >> > > > the wire. >> > > > >> > > >> > > In the wire protocol, we would still use a size of -1 like we >> normally do >> > > for nullable strings and nullable bytes. So, it's still a bit magical, >> > but >> > > efficient and associated with the right field (which avoids some >> invalid >> > > states that are possible if we use two fields). In other words, each >> > > implementation of the protocol is responsible for figuring out an >> > idiomatic >> > > and hopefully safe way to represent the absence of a value. >> > > >> > > In Java (Scala would be similar) we would convert this to an >> > > Optional<List<String>> to make it clear that the value could be absent >> > (and >> > > avoid NPEs). The fact that absence of a value means "all topics" makes >> > > sense if one thinks about that field as a filter (absence of a value >> > means >> > > no filter). >> > > >> > > I can see pros and cons for each approach, personally. :) >> > > >> > > Ismael >> > > >> > >> > > > > -- > 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
