Thanks, folks. The KIP appears to be satisfactory, so I plan to start a vote thread tomorrow unless there are objections.
Best regards, Randall On Mon, May 4, 2020 at 2:56 PM Christopher Egerton <chr...@confluent.io> wrote: > Hi Randall, > > Looks great! Definitely in favor of a WARN level message instead of failing > on startup. +1 non-binding when the vote thread comes > > Cheers, > > Chris > > On Mon, May 4, 2020 at 12:51 PM Randall Hauch <rha...@gmail.com> wrote: > > > 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 > > > > > > > > > > > > > > >
