On 2019/01/24 17:26:02, Andy Coates <a...@confluent.io> wrote:
> I'm wondering why we're rejected changing AbstractConfig to automatically
> resolve the variables?
>
> > 1. Change AbstractConfig to *automatically* resolve variables of the form
> specified in KIP-297. This was rejected because it would change the
> behavior of existing code and might cause unexpected effects.
>
> Doing so seems to me to have two very large benefits:
>
> 1. It allows the config providers to be defined within the same file as the
> config that uses the providers, e.g.
>
> config.providers=file,vault
> <https://cwiki.apache.org/confluence/display/KAFKA/config.providers=file,vault>
> config.providers.file.
> <https://cwiki.apache.org/confluence/display/KAFKA/config.providers.file.>
> class=org.apache.kafka.connect.configs.FileConfigProvider
> <https://cwiki.apache.org/confluence/display/KAFKA/org.apache.kafka.connect.configs.FileConfigProvider>
> config.providers.file.param.path=
> <https://cwiki.apache.org/confluence/display/KAFKA/config.providers.file.other.prop=another>
> /mnt/secrets/passwords
>
> foo.baz=/usr/temp/
> <https://cwiki.apache.org/confluence/display/KAFKA/foo.baz=/usr/temp/>
> foo.bar=$ <https://cwiki.apache.org/confluence/display/KAFKA/foo.bar=$>
> {file:/path/to/variables.properties:foo.bar}
>
> Is this possible with what's currently being proposed? i.e could you load
> the file and pass the map first to `loadConfigProviders` and then again to
> the constructor?
>
> 2. It allows _all_ existing clients of the class, e.g. those in Apache
> Kafka or in applications written by other people that use the class, to get
> this functionality for free, i.e. without any code changes. (I realize
> this is probably where the 'unexpected effects' comes from).
>
> I'm assuming the unexpected side effects come about if an existing
> properties file already contains compatible config.providers
> <https://cwiki.apache.org/confluence/display/KAFKA/config.providers=file,vault>
> entries _and_ has other properties in the form ${xx:yy} or ${xx:yy:zz}.
> While possible, these seems fairly unlikely unless for random client
> property files. So I'm assuming there's a specific instance where we think
> this is likely? Something to do with Connect config maybe?
>
> Personally, I think we should do our best to make this work seamlessly /
> transparently, because we're likely going to have this functionality for a
> long time.
>
> Andy
>
>
>
>
> On Tue, 22 Jan 2019 at 17:38, te...@confluent.io <te...@confluent.io> wrote:
>
> > Hi all,
> >
> > We would like to start vote on KIP-421 to to enhance the AbstractConfig
> > base class to support replacing variables in configurations just prior to
> > parsing and validation.
> >
> > Link for the KIP:
> >
> >
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-421%3A+Support+resolving+externalized+secrets+in+AbstractConfig
> >
> >
> > Thanks,
> > Tejal
> >
> Hi Andy,
So wanted to make sure that we come up with a simple approach with no side
effects or additional changes to any components. The rejected approach would
require a change in Connect's behavior and we dint want to make that for this
approach.
also regarding Point 1. yes thats exactly the expected behavior of
loadConfigProviders, we will send a file to it and it will create the instances
of the configProvider which will be consumed by the constructor.
Thanks,
Tejal
