Jackie-Jiang commented on PR #10836: URL: https://github.com/apache/pinot/pull/10836#issuecomment-1577280103
> @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? That is basically the point I'm trying to make. We should not modify the config read from the ZK because that can cause confusion, and certain dependency on how the code is executed. The current code doesn't modify the config but the new code does, thus I'm suggesting not making this change. -- 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
