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
