Thanks, Chris. 1. Added a use case that describes why you might want to use -1 for replication factor but want to set other properties. 2&3. Thanks for bringing up these special properties that the worker always sets. I've added an "Excluded properties" column to the table of new properties, and listed out the topic-specific properties should not be set. I've also added this paragraph:
Note that some topic-specific properties are excluded because the distributed worker always sets specific values. Therefore, if a distributed worker configuration does set any of these excluded properties, the distributed worker will issue a warning that such properties should not be set and will be ignored. I'm proposing that using such properties will result in a WARN log message, but allow the worker to continue by ignoring these values. I added a rejected alternative describing why I think failing is not desirable. 4. Added a sentence as suggested. Thanks! On Sun, May 3, 2020 at 12:55 PM Christopher Egerton <chr...@confluent.io> wrote: > Hi Randall, > > Thanks for the KIP! I have a few questions and suggestions but no major > objections. > > 1. The motivation is pretty clear for altering the various > "*.storage.replication.factor" properties to allow -1 as a value now. Are > there expected use cases for allowing modification of other properties of > these topic configs? It'd be nice to understand why we're adding this extra > configurability to the worker. > > 2. Should the "cleanup.policy" property have some additional guarding logic > to make sure that people don't set it to "delete" or "both"? > > 3. The lack of a "config.storage.partitions" property seems intentional > because the config topic should only ever have one partition. Now that > we're adding all of these other internal topic-related properties, do you > think it might be helpful to users if we emit a warning message of some > sort when they try to configure their worker with this property? > > 4. On the topic of compatibility--this is a fairly niche edge case, but any > time we add new configs to the worker we run the risk of overlap with > existing configs for REST extensions that users may have implemented. This > is different from other pluggable interfaces like config providers and > converters, whose properties are namespaced (presumably to avoid collisions > like this). Might be worth it to note this in a small paragraph or even > just a single sentence. > > Cheers, > > Chris > > On Thu, Apr 30, 2020 at 4:32 PM Ryanne Dolan <ryannedo...@gmail.com> > wrote: > > > Much needed, thanks. > > > > Ryanne > > > > On Thu, Apr 30, 2020 at 4:59 PM Randall Hauch <rha...@gmail.com> wrote: > > > > > Hello! > > > > > > I'd like to use this thread to discuss KIP-605, which expands some of > the > > > properties that the Connect distributed worker uses when creating > > internal > > > topics: > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-605%3A+Expand+Connect+Worker+Internal+Topic+Settings > > > > > > Best regards, > > > > > > Randall > > > > > >
