[GitHub] [pinot] KKcorps commented on pull request #10836: refactor ConcurrentMapPartitionUpsertMetadataManager constructor

["KKcorps (via GitHub)" <gi...@apache.org>]

KKcorps commented on PR #10836:
URL: https://github.com/apache/pinot/pull/10836#issuecomment-1578005340

   > > @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


-- 
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-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org