Copilot commented on code in PR #9499: URL: https://github.com/apache/gravitino/pull/9499#discussion_r2638411294
########## catalogs/catalog-hive/src/test/java/org/apache/gravitino/catalog/hive/integration/test/CatalogHive3ITWithCatalog.java: ########## @@ -0,0 +1,72 @@ +/* + * 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.gravitino.catalog.hive.integration.test; + +import static org.apache.gravitino.catalog.hive.HiveConstants.DEFAULT_CATALOG; +import static org.apache.gravitino.catalog.hive.HiveConstants.METASTORE_URIS; + +import com.google.common.collect.Maps; +import java.util.Map; +import org.apache.gravitino.Catalog; +import org.apache.gravitino.integration.test.container.HiveContainer; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.TestInstance; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +@Tag("gravitino-docker-test") +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +public class CatalogHive3ITWithCatalog extends CatalogHive3IT { + + private static final Logger LOGGER = LoggerFactory.getLogger(CatalogHiveS3IT.class); + + protected void startNecessaryContainer() { + super.startNecessaryContainer(); + hmsCatalog = "mycatalog"; + enableSparkTest = false; + } + + @Override + protected void createCatalog() { + + String localtion = Review Comment: Typo in variable name: "localtion" should be "location". ########## catalogs/catalog-hive/src/test/java/org/apache/gravitino/catalog/hive/integration/test/CatalogHive3ITWithCatalog.java: ########## @@ -0,0 +1,72 @@ +/* + * 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.gravitino.catalog.hive.integration.test; + +import static org.apache.gravitino.catalog.hive.HiveConstants.DEFAULT_CATALOG; +import static org.apache.gravitino.catalog.hive.HiveConstants.METASTORE_URIS; + +import com.google.common.collect.Maps; +import java.util.Map; +import org.apache.gravitino.Catalog; +import org.apache.gravitino.integration.test.container.HiveContainer; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.TestInstance; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +@Tag("gravitino-docker-test") +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +public class CatalogHive3ITWithCatalog extends CatalogHive3IT { + + private static final Logger LOGGER = LoggerFactory.getLogger(CatalogHiveS3IT.class); Review Comment: Logger is initialized with the wrong class name. It should use `CatalogHive3ITWithCatalog.class` instead of `CatalogHiveS3IT.class`. ```suggestion private static final Logger LOGGER = LoggerFactory.getLogger(CatalogHive3ITWithCatalog.class); ``` ########## catalogs/hive-metastore-common/src/main/java/org/apache/gravitino/hive/client/HiveShim.java: ########## @@ -108,6 +108,8 @@ public abstract List<HiveTable> getTableObjectsByName( public abstract List<String> getCatalogs(); Review Comment: Missing Javadoc for the new abstract method createCatalog. Since this is an abstract method that implementations must override, it should have complete documentation including parameter descriptions and exception behavior. ```suggestion /** * Creates a new catalog in the underlying Hive metastore. * * @param catalogName The name of the catalog to create. * @param location The default location URI for the catalog's data. * @param description A human-readable description of the catalog. * @throws RuntimeException if the catalog cannot be created for any reason, for example if a * catalog with the same name already exists or the metastore is unreachable. */ ``` ########## catalogs/catalog-hive/src/test/java/org/apache/gravitino/catalog/hive/integration/test/CatalogHive3ITWithCatalog.java: ########## @@ -0,0 +1,72 @@ +/* + * 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.gravitino.catalog.hive.integration.test; + +import static org.apache.gravitino.catalog.hive.HiveConstants.DEFAULT_CATALOG; +import static org.apache.gravitino.catalog.hive.HiveConstants.METASTORE_URIS; + +import com.google.common.collect.Maps; +import java.util.Map; +import org.apache.gravitino.Catalog; +import org.apache.gravitino.integration.test.container.HiveContainer; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.TestInstance; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +@Tag("gravitino-docker-test") +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +public class CatalogHive3ITWithCatalog extends CatalogHive3IT { + + private static final Logger LOGGER = LoggerFactory.getLogger(CatalogHiveS3IT.class); + + protected void startNecessaryContainer() { + super.startNecessaryContainer(); + hmsCatalog = "mycatalog"; + enableSparkTest = false; + } Review Comment: Missing @Override annotation on the startNecessaryContainer method that overrides a protected method from the parent class. ########## catalogs/catalog-hive/src/test/java/org/apache/gravitino/catalog/hive/integration/test/CatalogHive3ITWithCatalog.java: ########## @@ -0,0 +1,72 @@ +/* + * 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.gravitino.catalog.hive.integration.test; + +import static org.apache.gravitino.catalog.hive.HiveConstants.DEFAULT_CATALOG; +import static org.apache.gravitino.catalog.hive.HiveConstants.METASTORE_URIS; + +import com.google.common.collect.Maps; +import java.util.Map; +import org.apache.gravitino.Catalog; +import org.apache.gravitino.integration.test.container.HiveContainer; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.TestInstance; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +@Tag("gravitino-docker-test") +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +public class CatalogHive3ITWithCatalog extends CatalogHive3IT { + + private static final Logger LOGGER = LoggerFactory.getLogger(CatalogHiveS3IT.class); + + protected void startNecessaryContainer() { + super.startNecessaryContainer(); + hmsCatalog = "mycatalog"; + enableSparkTest = false; + } + + @Override + protected void createCatalog() { + + String localtion = + String.format( + "hdfs://%s:%d/user/hive/warehouse/" + hmsCatalog, + containerSuite.getHiveContainer().getContainerIpAddress(), + HiveContainer.HDFS_DEFAULTFS_PORT, + schemaName.toLowerCase()); Review Comment: The String.format call includes a third placeholder in the format string but only two arguments are provided. The third argument 'schemaName.toLowerCase()' appears in the format string but there's no %s for it. The format string should either include the schemaName placeholder or it should be removed from the arguments. ```suggestion HiveContainer.HDFS_DEFAULTFS_PORT); ``` ########## catalogs/hive-metastore-common/src/main/java/org/apache/gravitino/hive/client/HiveClient.java: ########## @@ -85,6 +85,10 @@ List<HiveTable> getTableObjectsByName( List<String> getCatalogs(); + void createCatalog(String catalogName, String location); + Review Comment: Missing Javadoc for the new public API methods createCatalog. Since these are public interface methods, they should have complete documentation including parameter descriptions, return value (if any), and exception behavior. ```suggestion /** * Creates a new Hive catalog at the specified storage location. * * @param catalogName The name of the catalog to create. * @param location The default storage location URI for the catalog. * @throws RuntimeException If the catalog already exists or the creation operation fails. */ void createCatalog(String catalogName, String location); /** * Creates a new Hive catalog at the specified storage location with a description. * * @param catalogName The name of the catalog to create. * @param location The default storage location URI for the catalog. * @param description A human-readable description of the catalog. * @throws RuntimeException If the catalog already exists or the creation operation fails. */ ``` -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
