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

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

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

   @Jackie-Jiang Is an instance of `UpsertConfig` meant to be mutable after its creation? Meaning once the upsert config object is instantiated, it seems acceptable to override a member variable with the setters. 
   
   I don't think this is a good practice, in general, as a config should be immutable once created. It might be convenient for testing purposes. Can we follow a builder pattern with `UpsertConfig` if it makes sense? Or at least, disallow setters on config objects? 


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