xuefuz 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-498877524 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.
---------------------------------------------------------------- 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: [email protected] With regards, Apache Git Services
