Hi Luis, I believe that compaction property is indeed overridable at per-topic level, as in
https://github.com/apache/kafka/blob/0cacbcf30e0a90ab9fad7bc310e5477cf959f1fd/clients/src/main/java/org/apache/kafka/common/config/TopicConfig.java#L116 And also documented in https://kafka.apache.org/documentation/#topicconfigs Is that right? Guozhang On Mon, Jul 2, 2018 at 7:41 AM, Luís Cabral <luis_cab...@yahoo.com.invalid> wrote: > Hi Guozhang, > > You are right that it is not straightforward to add a dependent property > validation. > Though it is possible to re-design it to allow for this, that effort would > be better placed under its own KIP, if it really becomes useful for other > properties as well. > Given this, the fallback-to-offset behaviour currently documented will be > used. > > Also, while analyzing this, I noticed that the existing compaction > properties only exist globally, and not per topic. > I don't understand why this is, but it again feels like something out of > scope for this KIP. > Given this, the KIP was updated to only include the global configuration > properties, removing the per-topic configs. > > I'll soon update the PR according to the documentation, but I trust the > KIP doesn't need that to close, right? > > Cheers, > Luis > > On Monday, July 2, 2018, 2:00:08 PM GMT+2, Luís Cabral > <luis_cab...@yahoo.com.INVALID> wrote: > > Hi Guozhang, > > At the moment the KIP has your vote, Matthias' and Ted's. > Should I ask someone else to have a look? > > Cheers, > Luis > > On Monday, July 2, 2018, 12:16:48 PM GMT+2, Mickael Maison < > mickael.mai...@gmail.com> wrote: > > +1 (non binding). Thanks for the KIP! > > On Sat, Jun 30, 2018 at 12:26 AM, Guozhang Wang <wangg...@gmail.com> > wrote: > > Hi Luis, > > > > Regarding the minor suggest, I agree it would be better to make it as > > mandatory, but it might be a bit tricky because it is a conditional > > mandatory one depending on the other config's value. Would like to see > your > > updated PR. > > > > Regarding the KIP itself, both Matthias and myself can recast our votes > to > > the updated wiki, while we still need one more committer to vote > according > > to the bylaws. > > > > > > Guozhang > > > > On Thu, Jun 28, 2018 at 5:38 AM, Luís Cabral > <luis_cab...@yahoo.com.invalid> > > wrote: > > > >> Hi, > >> > >> Thank you all for having a look! > >> > >> The KIP is now updated with the result of these late discussions, > though I > >> did take some liberty with this part: > >> > >> > >> - If the "compaction.strategy.header" configuration is not set (or is > >> blank), then the compaction strategy will fallback to "offset"; > >> > >> > >> Alternatively, we can also set it to be a mandatory property when the > >> strategy is "header" and fail the application to start via a config > >> validation (I would honestly prefer this, but its up to your taste). > >> > >> Anyway, this is now a minute detail that can be adapted during the final > >> stage of this KIP, so are you all alright with me changing the status to > >> [ACCEPTED]? > >> > >> Cheers, > >> Luis > >> > >> > >> On Thursday, June 28, 2018, 2:08:11 PM GMT+2, Ted Yu < > >> yuzhih...@gmail.com> wrote: > >> > >> +1 > >> > >> On Thu, Jun 28, 2018 at 4:56 AM, Luís Cabral > <luis_cab...@yahoo.com.invalid > >> > > >> wrote: > >> > >> > Hi Ted, > >> > Can I also get your input on this? > >> > > >> > bq. +1 from my side for using `compaction.strategy` with values > >> > "offset","timestamp" and "header" and `compaction.strategy.header` > >> > -Matthias > >> > > >> > bq. +1 from me as well. > >> > -Guozhang > >> > > >> > > >> > Cheers, > >> > Luis > >> > > >> > > >> > > >> > > > > > > > > -- > > -- Guozhang > -- -- Guozhang
