UOETianleZhang commented on PR #15799: URL: https://github.com/apache/pinot/pull/15799#issuecomment-2895306445
> I do agree with the general intention, beyond the config package discussed here I personally don't see many other exception cases. > > My concern is that for actively developed config classes (e.g. SchemaConformingTransformer) we'll quickly end up in a situation with dozens of constructors in the classes/hundreds of lines devoted to this. For consumers of the spi I don't think this provides a great experience either - to choose between so many ctors leads to confusion and misuse as users generally choose a simple ctor and don't think about what's required to create the object they need. > > It's also cumbersome and error prone to update all of these for each change. Will contributors expected to test each new constructor? It promotes poor practice such as using Map<String, String> for configs (e.g. FieldConfig properties) since it works around this new limitation. > > What do you think about exempting configs (again, just JSON configs), but also adding builders to the exempted configs such that the spi can be consumed in a way that constructor changes to these classes do not require any code change. IIUC that would prevent the build breaking changes you refer to, and also allow for simpler development process/better maintainability in the long term. @itschrispeck Thank you for providing your thoughtful feedback. We are having an internal discussion and will respond later. I’m also drafting a complementary proposal and will open a doc/issue later this week or early next week for broader discussion :) cc @siddharthteotia -- 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
