Great, looking good. I'd probably be a bit more concrete about the Proposed Changes (e.g., "will log an warning if the config is specified" and "since the JsonConverter is the default, the configs will be removed immediately from the example worker configuration files").
Other than that this LGTM and I'll be happy to get rid of those settings! -Ewen On Tue, Aug 8, 2017 at 2:54 AM, UMESH CHAUDHARY <umesh9...@gmail.com> wrote: > Hi Ewen, > Sorry, I am bit late in responding this. > > Thanks for your inputs and I've updated the KIP by adding more details to > it. > > Regards, > Umesh > > On Mon, 31 Jul 2017 at 21:51 Ewen Cheslack-Postava <e...@confluent.io> > wrote: > >> On Sun, Jul 30, 2017 at 10:21 PM, UMESH CHAUDHARY <umesh9...@gmail.com> >> wrote: >> >>> Hi Ewen, >>> Thanks for your comments. >>> >>> 1) Yes, there are some test and java classes which refer these configs, >>> so I will include them as well in "public interface" section of KIP. What >>> should be our approach to deal with the classes and tests which use these >>> configs: we need to change them to use JsonConverter when we plan for >>> removal of these configs right? >>> >> >> I actually meant the references in config/connect-standalone.properties >> and config/connect-distributed.properties >> >> >>> 2) I believe we can target the deprecation in 1.0.0 release as it is >>> planned in October 2017 and then removal in next major release. Let me >>> know your thoughts as we don't have any information for next major release >>> (next to 1.0.0) yet. >>> >> >> That sounds fine. Tough to say at this point what our approach to major >> version bumps will be since the approach to version numbering is changing a >> bit. >> >> >>> 3) Thats a good point and mentioned JIRA can help us to validate the >>> usage of any other converters. I will list this down in the KIP. >>> >>> Let me know if you have some additional thoughts on this. >>> >>> Regards, >>> Umesh >>> >>> >>> >>> On Wed, 26 Jul 2017 at 09:27 Ewen Cheslack-Postava <e...@confluent.io> >>> wrote: >>> >>>> Umesh, >>>> >>>> Thanks for the KIP. Straightforward and I think it's a good change. >>>> Unfortunately it is hard to tell how many people it would affect since >>>> we >>>> can't tell how many people have adjusted that config, but I think this >>>> is >>>> the right thing to do long term. >>>> >>>> A couple of quick things that might be helpful to refine: >>>> >>>> * Note that there are also some references in the example configs that >>>> we >>>> should remove. >>>> * It's nice to be explicit about when the removal is planned. This lets >>>> us >>>> set expectations with users for timeframe (especially now that we have >>>> time >>>> based releases), allows us to give info about the removal timeframe in >>>> log >>>> error messages, and lets us file a JIRA against that release so we >>>> remember >>>> to follow up. Given the update to 1.0.0 for the next release, we may >>>> also >>>> need to adjust how we deal with deprecations/removal if we don't want to >>>> have to wait all the way until 2.0 to remove (though it is unclear how >>>> exactly we will be handling version bumps from now on). >>>> * Migration path -- I think this is the major missing gap in the KIP. >>>> Do we >>>> need a migration path? If not, presumably it is because people aren't >>>> using >>>> any other converters in practice. Do we have some way of validating >>>> this ( >>>> https://issues.apache.org/jira/browse/KAFKA-3988 might be pretty >>>> convincing >>>> evidence)? If there are some users using other converters, how would >>>> they >>>> migrate to newer versions which would no longer support that? >>>> >>>> -Ewen >>>> >>>> >>>> On Fri, Jul 14, 2017 at 2:37 AM, UMESH CHAUDHARY <umesh9...@gmail.com> >>>> wrote: >>>> >>>> > Hi there, >>>> > Resending as probably missed earlier to grab your attention. >>>> > >>>> > Regards, >>>> > Umesh >>>> > >>>> > ---------- Forwarded message --------- >>>> > From: UMESH CHAUDHARY <umesh9...@gmail.com> >>>> > Date: Mon, 3 Jul 2017 at 11:04 >>>> > Subject: [DISCUSS] KIP-174 - Deprecate and remove internal converter >>>> > configs in WorkerConfig >>>> > To: d...@kafka.apache.org <d...@kafka.apache.org> >>>> > >>>> > >>>> > Hello All, >>>> > I have added a KIP recently to deprecate and remove internal converter >>>> > configs in WorkerConfig.java class because these have ultimately just >>>> > caused a lot more trouble and confusion than it is worth. >>>> > >>>> > Please find the KIP here >>>> > <https://cwiki.apache.org/confluence/display/KAFKA/KIP- >>>> > 174+-+Deprecate+and+remove+internal+converter+configs+in+ >>>> WorkerConfig> >>>> > and >>>> > the related JIRA here <https://issues.apache.org/ >>>> jira/browse/KAFKA-5540>. >>>> > >>>> > Appreciate your review and comments. >>>> > >>>> > Regards, >>>> > Umesh >>>> > >>>> >>>
