bowenli86 commented on issue #8616: [FLINK-12718][hive] allow users to specify hive-site.xml location to configure hive metastore client in HiveCatalog URL: https://github.com/apache/flink/pull/8616#issuecomment-499174286 > Thanks for the contribution. I have a couple of high-level comments: > > 1. The PR seems suggesting that hive-site.xml path is always required, which shouldn't be the case. We should just instantiate a HiveConf instance and get the config dir from the environment. > 2. There seems to be a good number of constructors. We can probably reduce it by only providing Catalog(String catName, String dbName, String/URL hiveSiteLocation) and taking care of nulls. We can add more on demand later on. I was thinking that the two issues above can be solved by introducing a builder. I will keep just a few constructors required by the following PR for catalog entries, and add a builder later on
---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
