okumin commented on code in PR #5955: URL: https://github.com/apache/hive/pull/5955#discussion_r2203163029
########## standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/client/HiveMetaStoreClientFactory.java: ########## @@ -0,0 +1,58 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hadoop.hive.metastore.client; + +import org.apache.commons.lang3.exception.ExceptionUtils; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hive.metastore.IMetaStoreClient; +import org.apache.hadoop.hive.metastore.api.MetaException; +import org.apache.hadoop.hive.metastore.conf.MetastoreConf; +import org.apache.hadoop.hive.metastore.utils.JavaUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * A factory class creating a MetaStoreClient specified in a given configuration. + */ +public class HiveMetaStoreClientFactory { + private static final Logger LOG = LoggerFactory.getLogger(HiveMetaStoreClientFactory.class); + + public static IMetaStoreClient newClient(Configuration conf, boolean allowEmbedded) throws MetaException { + String mscClassName = MetastoreConf.getVar(conf, MetastoreConf.ConfVars.METASTORE_CLIENT_CLASS); Review Comment: Can we use MetastoreConf.getClass? ########## standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java: ########## @@ -1728,6 +1728,10 @@ public enum ConfVars { " and password. Any other value is ignored right now but may be used later." + "If JWT- Supported only in HTTP transport mode. If set, HMS Client will pick the value of JWT from " + "environment variable HMS_JWT and set it in Authorization header in http request"), + METASTORE_CLIENT_CLASS("metastore.client.class", + "hive.metastore.client.class", + "org.apache.hadoop.hive.metastore.client.ThriftHiveMetaStoreClient", + "The name of MetaStoreClient class that implements the IMetaStoreClient interface."), Review Comment: @moomindani Please feel free to give us a suggestion if you have any thoughts. > Regarding the design decision about MetaStoreClientFactory: This patch does not support a customizable MSC factory as the original patch did; instead, it supports a customizable MSC. In other words, this patch introduces metastore.client.class, not metastore.client.factory.class. I made this change not because I preferred this approach, but because I followed the majority opinion in the discussion https://github.com/apache/hive/pull/4444#pullrequestreview-1492711587. Personally, I think both approaches are valid and have their merits, and I do not have a strong opinion among them. I would appreciate it if you could share your thoughts on this decision. -- 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: gitbox-unsubscr...@hive.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
