Re: [DISCUSS] KIP-569: DescribeConfigsResponse - Update the schema to include datatype of the field

[Shailesh Panwar <sp...@confluent.io>]

I have updated the KIP to incorporate the feedback received so far. Changes
include
1. By default, the new fields would not be included in the response. Client
can include them via DescribeConfigsOptions
2. Updated the Compatibility section with backward and forward
compatibility behaviour.

https://cwiki.apache.org/confluence/display/KAFKA/KIP-569%3A+DescribeConfigsResponse+-+Update+the+schema+to+include+additional+metadata+information+of+the+field

Thanks
Shailesh

On Fri, Feb 28, 2020 at 3:18 PM Colin McCabe <cm...@apache.org> wrote:

> On Fri, Feb 28, 2020, at 14:00, Shailesh Panwar wrote:
> > On Fri, Feb 28, 2020 at 11:04 AM Colin McCabe <cm...@apache.org>
> wrote:
> >
> > > On Thu, Feb 27, 2020, at 11:37, Shailesh Panwar wrote:
> > > > Thanks Colin for the inputs. Regarding
> > > >
> > > > > I think most users of the DescribeConfigs API do not want to get
> help
> > > text
> > > > > or configuration schema information.
> > > > There is already a precedence of including this information as part
> of
> > > > Connect Config - both type and documentation - and we have found it
> quite
> > > > useful in the Config client forms.
> > >
> > > Hi Shailesh,
> > >
> > > I think you make a good point that this information is useful.  My
> point
> > > is just that we shouldn't send this information unless it is really
> needed,
> > > since it is quite large in size.
> > >
> > >
> > [shailesh]: I agree. Including documentation would bloat the response esp
> > if there are clients that don't care about this information. One way I
> can
> > think of mitigating this is by giving clients the option to include it -
> > similar to DescribeConfigsOptions.includeSynonyms. I'll update the KIP if
> > this approach sounds correct.
> >
>
> Right, I'm glad you see the issue.  I guess my feeling is that if we're
> going to create a new API for listing configuration metadata anyway, then
> we don't need to make DescribeConfigs more complex.  But I'm curious what
> others think here.
>
> > >
> > > > > It would be better to create a new, separate API for getting this
> > > > > configuration schema information, if that's what we really want.
> This
> > > API
> > > > > should probably also allow configuration management systems to list
> > > all the
> > > > > possible configurations for topics, brokers, etc., which is
> something
> > > that
> > > > > I think many of them would want.
> > > >
> > > > Correct. I believe AdminClient.describeConfig already provides us
> that
> > > > today. From the docs - "Get the configuration for the specified
> resources
> > > > with the default options."
> > > > This api gives us back all the configuration information for the
> > > specified
> > > > resources(s) - broker or  topic.
> > > >
> > >
> > > Just to give an example, let's say your management system wants to know
> > > about all possible topic configurations.  I don't think you can get
> that
> > > from describeConfigs today, since it will only show you the topic
> configs
> > > that are actually set.
> > >
> > > But let's suppose it did list all possible topic configurations.  It
> would
> > > be weird to have to create a "fake" topic just so you could call
> > > describeConfigs on it, to find out all the potential configurations.  I
> > > think this highlights the fact that listing potential configurations
> is a
> > > different operation from listing the currently configured ones, and
> > > deserves its own API.
> > >
> > >
> > [shailesh]: Makes sense, and I agree there should be a better way (new
> api)
> > to list all the Topic(and other resource) configurations. In fact, in the
> > case of topics, the way client has been fetching all the configs is by
> > doing exactly as you described above. Create a topic with
> name=random-guid
> > and validateOnly=true.
>
> Thanks for the context.  It would be good to make that hack unnecessary :)
>
> best,
> Colin
>
> > To accomplish this, we can break down this problem into 2 different KIPs
> -
> > 1st Kip to include the new fields (this one) and 2nd Kip would be
> focussed
> > on creating the new API. Both would be independent in the sense that the
> > 2nd Kip would automatically get the new fields as I believe the response
> > schema would be shared.
> >
> > >
> > > > > We also need to consider compatibility.  One example is, what if we
> > > later
> > > > > add a new type of configuration key, such as UUID.  What would the
> > > > > hypothetical DescribeConfigurationSchema API return in this case,
> for
> > > older
> > > > > clients?  We probably need an UNKNOWN enum value to be used to
> indicate
> > > > > that the server knows about configuration key types that the client
> > > does
> > > > > not.
> > > >
> > > > I believe we already handle backward compatibility via versioning on
> the
> > > > message schema. For example 'Synonyms' is only available from 1+
> version
> > > > and treated nullable for previous ones. The expectation with the new
> > > fields
> > > > is similar.
> > >
> > > The point I was making is that there will be some version X client that
> > > doesn't understand all the possible configuration types that a version
> X +
> > > 1 client and broker understand.  What does the version X client see in
> this
> > > case when it uses the API?
> > >
> > >
> > [shailesh]: Ah I see your point. UNKNOWN enum value would make sense in
> > this case. I'll update 'Compatibility' section in the Kip.
> >
> > best,
> > > Colin
> > >
> > > >
> > > > Thanks
> > > > Shailesh
> > > >
> > > > On 2020/02/26 22:17:39, "Colin McCabe" <c....@apache.org> wrote:
> > > > > Hi Shailesh,>
> > > > >
> > > > > I think most users of the DescribeConfigs API do not want to get
> help
> > > > text or configuration schema information.  So, it would be
> inefficient to
> > > > always include this information as part of the DescribeConfigs
> response.
> > > > It would be better to create a new, separate API for getting this
> > > > configuration schema information, if that's what we really want.
> This
> > > API
> > > > should probably also allow configuration management systems to list
> all
> > > the
> > > > possible configurations for topics, brokers, etc., which is something
> > > that
> > > > I think many of them would want.>
> > > > >
> > > > > We also need to consider compatibility.  One example is, what if we
> > > later
> > > > add a new type of configuration key, such as UUID.  What would the
> > > > hypothetical DescribeConfigurationSchema API return in this case, for
> > > older
> > > > clients?  We probably need an UNKNOWN enum value to be used to
> indicate
> > > > that the server knows about configuration key types that the client
> does
> > > > not.>
> > > > >
> > > > > best,>
> > > > > Colin>
> > > > >
> > > > >
> > > > > On Wed, Feb 19, 2020, at 09:13, Shailesh Panwar wrote:>
> > > > > > Bump.>
> > > > > > >
> > > > > > Thanks>
> > > > > > Shailesh>
> > > > > > >
> > > > > > On Tue, Feb 11, 2020 at 1:00 PM Shailesh Panwar <
> sp...@confluent.io
> > > >>
> > > > > > wrote:>
> > > > > > >
> > > > > > > Hi all,>
> > > > > > > We would like to extend the DescribeConfigsResponse schema to
> > > include
> > > > the>
> > > > > > > data-type of the fields.>
> > > > > > >>
> > > > > > > The KIP can be found here:>
> > > > > > >>
> > > > > > >
> > > >
> > >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-569%3A+DescribeConfigsResponse+-+Update+the+schema+to+include+datatype+of+the+field
> > > >
> > > >
> > > > > > >>
> > > > > > > Thanks>
> > > > > > > Shailesh>
> > > > > > >>
> > > > > >>
> > > > >
> > > >
> > >
> >
>