Thanks a lot, Colin. This KIP has now passed voting with 3 binding votes ( Randall, Rajini & Colin) and 1 non-binding vote (Chris). Thanks a lot, everyone for the feedback & discussion on this KIP.
On Fri, May 10, 2019 at 9:12 AM Colin McCabe <cmcc...@apache.org> wrote: > +1 (binding). Thanks, Magesh. > > cheers, > Colin > > On Thu, May 9, 2019, at 18:31, Randall Hauch wrote: > > I'm still +1 and like the simplification. > > > > Randall > > > > On Thu, May 9, 2019 at 5:54 PM Magesh Nandakumar <mage...@confluent.io> > > wrote: > > > > > I have updated the KIP to remove the `Ignore` policy and also the > > > useOverrides() > > > method in the interface. > > > Thanks a lot for your thoughts, Colin. I believe this certainly > simplifies > > > the KIP. > > > > > > On Thu, May 9, 2019 at 3:44 PM Magesh Nandakumar <mage...@confluent.io > > > > > wrote: > > > > > > > Unless anyone has objections, I'm going to update the KIP to remove > the > > > > `Ignore` policy and make `None` as the default. I will also remove > the ` > > > > default boolean useOverrides()` in the interface which was > introduced for > > > > the purpose of backward compatibility. > > > > > > > > On Thu, May 9, 2019 at 3:27 PM Randall Hauch <rha...@gmail.com> > wrote: > > > > > > > >> I have also seen users include in connector configs the > `producer.*` and > > > >> `consumer.*` properties that should go into the worker configs. But > > > those > > > >> don't match, and the likelihood that someone is already using > > > >> `producer.override.*` or `consumer.override.*` properties in their > > > >> connector configs does seem pretty tiny. > > > >> > > > >> I'd be fine with removing the `Ignore` for backward compatibility. > Still > > > >> +1 > > > >> either way. > > > >> > > > >> On Thu, May 9, 2019 at 5:23 PM Magesh Nandakumar < > mage...@confluent.io> > > > >> wrote: > > > >> > > > >> > To add more details regarding the backward compatibility; I have > > > >> generally > > > >> > seen users trying to set "producer.request.timeout.ms > > > >> > <http://producer.override.request.timeout.ms/>" in their > connector > > > >> config > > > >> > under the assumption that it will get used and would never come > back > > > to > > > >> > remove it. The initial intent of the KIP was to use the same > prefix > > > but > > > >> > since that potentially collided with MM2 configs, we agreed to > use a > > > >> > different prefix "producer.override". With this context, I think > the > > > >> > likelihood of someone using this is very small and should > generally > > > not > > > >> be > > > >> > a problem. > > > >> > > > > >> > On Thu, May 9, 2019 at 3:15 PM Magesh Nandakumar < > > > mage...@confluent.io> > > > >> > wrote: > > > >> > > > > >> > > Colin, > > > >> > > > > > >> > > Thanks a lot for the feedback. As you said, the possibilities > of > > > >> someone > > > >> > > having "producer.override.request.timeout.ms" in their > connector > > > >> config > > > >> > > in AK 2.2 or lower is very slim. But the key thing is if in > case, > > > >> someone > > > >> > > has it AK2.2 doesn't do anything with it and it silently > ignores the > > > >> > > configuration. If others think that it's not really a problem, > then > > > >> I'm > > > >> > > fine with removing the complicated compatibility issue. > > > >> > > > > > >> > > I have explicitly called out the behavior when the exception is > > > >> thrown. > > > >> > > > > > >> > > Let me know what you think. > > > >> > > > > > >> > > Thanks, > > > >> > > Magesh > > > >> > > > > > >> > > On Thu, May 9, 2019 at 2:45 PM Colin McCabe <cmcc...@apache.org > > > > > >> wrote: > > > >> > > > > > >> > >> Hi Magesh, > > > >> > >> > > > >> > >> Thanks for the KIP. It looks good overall. > > > >> > >> > > > >> > >> > default boolean useOverrides() { > > > >> > >> > return true; > > > >> > >> > } > > > >> > >> > > > >> > >> Is this method really needed? As I understand, nobody should > have > > > >> any > > > >> > >> connector client config overrides set right now, since they > don't > > > do > > > >> > >> anything right now. > > > >> > >> > > > >> > >> For example, you wouldn't expect a Kafka 2.2 installation to > have " > > > >> > >> producer.override.request.timeout.ms" set, since that doesn't > do > > > >> > >> anything in Kafka 2.2. So is the option to ignore it in Kafka > 2.3 > > > >> > really > > > >> > >> necessary? > > > >> > >> > > > >> > >> Can you add some details about what happens if a > > > >> > >> PolicyValidationException is thrown? I'm assuming that we > fail to > > > >> > create > > > >> > >> the new Connector, I'm not sure if that's completely spelled > out > > > >> > (unless I > > > >> > >> missed it). > > > >> > >> > > > >> > >> best, > > > >> > >> Colin > > > >> > >> > > > >> > >> > > > >> > >> On Thu, May 9, 2019, at 08:05, Rajini Sivaram wrote: > > > >> > >> > Hi Magesh, > > > >> > >> > > > > >> > >> > Thanks for the KIP, +1 (binding) > > > >> > >> > > > > >> > >> > Regards, > > > >> > >> > > > > >> > >> > Rajini > > > >> > >> > > > > >> > >> > > > > >> > >> > On Thu, May 9, 2019 at 3:55 PM Randall Hauch < > rha...@gmail.com> > > > >> > wrote: > > > >> > >> > > > > >> > >> > > Nice work, Magesh. > > > >> > >> > > > > > >> > >> > > +1 (binding) > > > >> > >> > > > > > >> > >> > > Randall > > > >> > >> > > > > > >> > >> > > On Wed, May 8, 2019 at 7:22 PM Magesh Nandakumar < > > > >> > >> mage...@confluent.io> > > > >> > >> > > wrote: > > > >> > >> > > > > > >> > >> > > > Thanks a lot Chris. So far, the KIP has one non-binding > vote > > > >> and > > > >> > I'm > > > >> > >> > > still > > > >> > >> > > > looking forward to the KIP to be voted by Friday's > deadline. > > > >> > >> > > > > > > >> > >> > > > On Tue, May 7, 2019 at 10:00 AM Chris Egerton < > > > >> > chr...@confluent.io> > > > >> > >> > > wrote: > > > >> > >> > > > > > > >> > >> > > > > Hi Magesh, > > > >> > >> > > > > > > > >> > >> > > > > This looks great! Very excited to see these changes > finally > > > >> > >> coming to > > > >> > >> > > > > Connect. > > > >> > >> > > > > +1 (non-binding) > > > >> > >> > > > > > > > >> > >> > > > > Cheers, > > > >> > >> > > > > > > > >> > >> > > > > Chris > > > >> > >> > > > > > > > >> > >> > > > > On Tue, May 7, 2019 at 9:51 AM Magesh Nandakumar < > > > >> > >> mage...@confluent.io > > > >> > >> > > > > > > >> > >> > > > > wrote: > > > >> > >> > > > > > > > >> > >> > > > > > Hi All, > > > >> > >> > > > > > > > > >> > >> > > > > > I would like to start a vote on > > > >> > >> > > > > > > > > >> > >> > > > > > > > > >> > >> > > > > > > > >> > >> > > > > > > >> > >> > > > > > >> > >> > > > >> > > > > >> > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-458%3A+Connector+Client+Config+Override+Policy > > > >> > >> > > > > > > > > >> > >> > > > > > The discussion thread can be found here > > > >> > >> > > > > > < > > > >> > >> > https://www.mail-archive.com/dev@kafka.apache.org/msg97124.html>. > > > >> > >> > > > > > > > > >> > >> > > > > > Thanks, > > > >> > >> > > > > > Magesh > > > >> > >> > > > > > > > > >> > >> > > > > > > > >> > >> > > > > > > >> > >> > > > > > >> > >> > > > > >> > >> > > > >> > > > > > >> > > > > >> > > > > > > > > > >
