Jackie-Jiang commented on PR #10836: URL: https://github.com/apache/pinot/pull/10836#issuecomment-1579206540
> > > @Jackie-Jiang Can I assign the `comparisonColumns` value to upsertConfig and pass it to PartitionUpsertMetadataManager? then we don't need to repeat the same steps in partitionlevel upsert metadata managers. > > > The purpose of the change is to address a comment from @KKcorps. [#10047 (comment)](https://github.com/apache/pinot/pull/10047#discussion_r1189404582) > > > > > > That can fix the issue, but also means it is harder to manage (next person might make the same mistake). I'd suggest keeping the constructor as is without overriding the config. @KKcorps wdyt? > > Yeah overriding the configs is a big no no. They are supposed to be immutable. > > I do want this constructor to be cleaned though using some way. > > Otherwise more and more args will just keep on adding here One way is to create a different wrapper class to hold the modified configs -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
