Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#4107] feat(all): Add testConnection API for catalog #4108

Merged
merged 4 commits into from
Jul 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions api/src/main/java/org/apache/gravitino/SupportsCatalogs.java
Original file line number Diff line number Diff line change
Expand Up @@ -114,4 +114,22 @@ Catalog alterCatalog(String catalogName, CatalogChange... changes)
* @return True if the catalog was dropped, false otherwise.
*/
boolean dropCatalog(String catalogName);

/**
* Test whether the catalog with specified parameters can be connected to before creating it.
*
* @param catalogName the name of the catalog.
* @param type the type of the catalog.
* @param provider the provider of the catalog.
* @param comment the comment of the catalog.
* @param properties the properties of the catalog.
* @throws Exception if the test failed.
*/
void testConnection(
String catalogName,
Catalog.Type type,
String provider,
String comment,
Map<String, String> properties)
throws Exception;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/*
* 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.exceptions;

import com.google.errorprone.annotations.FormatMethod;
import com.google.errorprone.annotations.FormatString;

/** An exception thrown when connect to catalog failed. */
public class ConnectionFailedException extends GravitinoRuntimeException {
/**
* Constructs a new exception with the specified detail message.
*
* @param cause the cause.
* @param errorMessageTemplate the detail message.
* @param args the arguments to the message.
*/
@FormatMethod
public ConnectionFailedException(
Throwable cause, @FormatString String errorMessageTemplate, Object... args) {
super(cause, errorMessageTemplate, args);
}

/**
* Constructs a new exception with the specified detail message.
*
* @param errorMessageTemplate the detail message.
* @param args the arguments to the message.
*/
@FormatMethod
public ConnectionFailedException(@FormatString String errorMessageTemplate, Object... args) {
super(errorMessageTemplate, args);
}
}
4 changes: 3 additions & 1 deletion build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -490,7 +490,9 @@ tasks.rat {
"clients/client-python/.venv/*",
"clients/client-python/gravitino.egg-info/*",
"clients/client-python/gravitino/utils/exceptions.py",
"clients/client-python/gravitino/utils/http_client.py"
"clients/client-python/gravitino/utils/http_client.py",
"clients/client-python/tests/unittests/htmlcov/*",
"clients/client-python/tests/integration/htmlcov/*"
)

// Add .gitignore excludes to the Apache Rat exclusion list.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import java.util.Optional;
import java.util.stream.Collectors;
import org.apache.commons.lang3.StringUtils;
import org.apache.gravitino.Catalog;
import org.apache.gravitino.Entity;
import org.apache.gravitino.EntityStore;
import org.apache.gravitino.GravitinoEnv;
Expand Down Expand Up @@ -538,6 +539,26 @@ public boolean dropSchema(NameIdentifier ident, boolean cascade) throws NonEmpty
}
}

/**
* Since the Hadoop catalog was completely managed by Gravitino, we don't need to test the
* connection
*
* @param catalogIdent the name of the catalog.
* @param type the type of the catalog.
* @param provider the provider of the catalog.
* @param comment the comment of the catalog.
* @param properties the properties of the catalog.
*/
@Override
public void testConnection(
NameIdentifier catalogIdent,
Catalog.Type type,
String provider,
String comment,
Map<String, String> properties) {
// Do nothing
}

@Override
public void close() throws IOException {}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
import java.util.stream.Collectors;
import java.util.stream.Stream;
import org.apache.commons.io.FileUtils;
import org.apache.gravitino.Catalog;
import org.apache.gravitino.Config;
import org.apache.gravitino.EntityStore;
import org.apache.gravitino.EntityStoreFactory;
Expand Down Expand Up @@ -703,6 +704,19 @@ public void testRemoveFilesetComment() throws IOException {
}
}

@Test
public void testTestConnection() {
HadoopCatalogOperations catalogOperations = new HadoopCatalogOperations(store);
Assertions.assertDoesNotThrow(
() ->
catalogOperations.testConnection(
NameIdentifier.of("metalake", "catalog"),
Catalog.Type.FILESET,
"hadoop",
"comment",
ImmutableMap.of()));
}

private static Stream<Arguments> locationArguments() {
return Stream.of(
// Honor the catalog location
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import org.apache.gravitino.Schema;
import org.apache.gravitino.client.GravitinoMetalake;
import org.apache.gravitino.exceptions.FilesetAlreadyExistsException;
import org.apache.gravitino.exceptions.IllegalNameIdentifierException;
import org.apache.gravitino.exceptions.NoSuchFilesetException;
import org.apache.gravitino.file.Fileset;
import org.apache.gravitino.file.FilesetChange;
Expand Down Expand Up @@ -187,7 +188,7 @@ public void testCreateFileset() throws IOException {

// create fileset with null fileset name
Assertions.assertThrows(
IllegalArgumentException.class,
IllegalNameIdentifierException.class,
() ->
createFileset(
null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import org.apache.gravitino.Schema;
import org.apache.gravitino.client.GravitinoMetalake;
import org.apache.gravitino.exceptions.FilesetAlreadyExistsException;
import org.apache.gravitino.exceptions.IllegalNameIdentifierException;
import org.apache.gravitino.file.Fileset;
import org.apache.gravitino.integration.test.util.AbstractIT;
import org.apache.gravitino.integration.test.util.GravitinoITUtils;
Expand Down Expand Up @@ -398,7 +399,7 @@ public void testCreateFileset() throws Exception {

// create fileset with null fileset name
Assertions.assertThrows(
IllegalArgumentException.class,
IllegalNameIdentifierException.class,
() ->
createFileset(
null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
import java.util.stream.Collectors;
import org.apache.commons.lang3.ArrayUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.gravitino.Catalog;
import org.apache.gravitino.NameIdentifier;
import org.apache.gravitino.Namespace;
import org.apache.gravitino.SchemaChange;
Expand All @@ -64,6 +65,7 @@
import org.apache.gravitino.connector.HasPropertyMetadata;
import org.apache.gravitino.connector.ProxyPlugin;
import org.apache.gravitino.connector.SupportsSchemas;
import org.apache.gravitino.exceptions.ConnectionFailedException;
import org.apache.gravitino.exceptions.NoSuchCatalogException;
import org.apache.gravitino.exceptions.NoSuchSchemaException;
import org.apache.gravitino.exceptions.NoSuchTableException;
Expand All @@ -85,6 +87,7 @@
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.hive.conf.HiveConf;
import org.apache.hadoop.hive.conf.HiveConf.ConfVars;
import org.apache.hadoop.hive.metastore.IMetaStoreClient;
import org.apache.hadoop.hive.metastore.api.AlreadyExistsException;
import org.apache.hadoop.hive.metastore.api.Database;
import org.apache.hadoop.hive.metastore.api.FieldSchema;
Expand Down Expand Up @@ -1091,6 +1094,30 @@ public boolean purgeTable(NameIdentifier tableIdent) throws UnsupportedOperation
}
}

/**
* Performs `getAllDatabases` operation in Hive Metastore to test the connection.
*
* @param catalogIdent the name of the catalog.
* @param type the type of the catalog.
* @param provider the provider of the catalog.
* @param comment the comment of the catalog.
* @param properties the properties of the catalog.
*/
@Override
public void testConnection(
NameIdentifier catalogIdent,
Catalog.Type type,
String provider,
String comment,
Map<String, String> properties) {
try {
clientPool.run(IMetaStoreClient::getAllDatabases);
} catch (Exception e) {
throw new ConnectionFailedException(
e, "Failed to run getAllDatabases in Hive Metastore: %s", e.getMessage());
}
}

/**
* Checks if the given namespace is a valid namespace for the Hive schema.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,20 @@
import static org.apache.gravitino.catalog.hive.HiveCatalogPropertiesMeta.PRINCIPAL;
import static org.apache.gravitino.catalog.hive.TestHiveCatalog.HIVE_PROPERTIES_METADATA;
import static org.apache.gravitino.connector.BaseCatalog.CATALOG_BYPASS_PREFIX;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Maps;
import java.util.Map;
import org.apache.gravitino.Catalog;
import org.apache.gravitino.NameIdentifier;
import org.apache.gravitino.connector.BaseCatalog;
import org.apache.gravitino.connector.PropertyEntry;
import org.apache.gravitino.exceptions.ConnectionFailedException;
import org.apache.hadoop.hive.conf.HiveConf.ConfVars;
import org.apache.thrift.TException;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;

Expand Down Expand Up @@ -126,4 +133,25 @@ void testPropertyOverwrite() {
Assertions.assertEquals("v2", op.hiveConf.get("a.b"));
Assertions.assertEquals("v4", op.hiveConf.get("c.d"));
}

@Test
void testTestConnection() throws TException, InterruptedException {
HiveCatalogOperations op = new HiveCatalogOperations();
op.clientPool = mock(CachedClientPool.class);
when(op.clientPool.run(any())).thenThrow(new TException("mock connection exception"));

ConnectionFailedException exception =
Assertions.assertThrows(
ConnectionFailedException.class,
() ->
op.testConnection(
NameIdentifier.of("metalake", "catalog"),
Catalog.Type.RELATIONAL,
"hive",
"comment",
ImmutableMap.of()));
Assertions.assertEquals(
"Failed to run getAllDatabases in Hive Metastore: mock connection exception",
exception.getMessage());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import java.util.stream.Stream;
import javax.sql.DataSource;
import org.apache.commons.lang3.StringUtils;
import org.apache.gravitino.Catalog;
import org.apache.gravitino.NameIdentifier;
import org.apache.gravitino.Namespace;
import org.apache.gravitino.SchemaChange;
Expand Down Expand Up @@ -173,6 +174,25 @@ public NameIdentifier[] listSchemas(Namespace namespace) throws NoSuchCatalogExc
.toArray(NameIdentifier[]::new);
}

/**
* Performs `show databases` operation to check if the JDBC connection is valid.
*
* @param catalogIdent the name of the catalog.
* @param type the type of the catalog.
* @param provider the provider of the catalog.
* @param comment the comment of the catalog.
* @param properties the properties of the catalog.
*/
@Override
public void testConnection(
NameIdentifier catalogIdent,
Catalog.Type type,
String provider,
String comment,
Map<String, String> properties) {
databaseOperation.listDatabases();
}

/**
* Creates a new schema with the provided identifier, comment and metadata.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ public abstract class JdbcExceptionConverter {
* Convert JDBC exception to GravitinoException.
*
* @param sqlException The sql exception to map
* @return A best attempt at a corresponding connector exception or generic with the SQLException
* as the cause
* @return The best attempt at a corresponding connector exception or generic with the
* SQLException as the cause
*/
@SuppressWarnings("FormatStringAnnotation")
public GravitinoRuntimeException toGravitinoException(final SQLException sqlException) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/*
* 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.jdbc;

import com.google.common.collect.ImmutableMap;
import org.apache.gravitino.Catalog;
import org.apache.gravitino.NameIdentifier;
import org.apache.gravitino.catalog.jdbc.converter.SqliteColumnDefaultValueConverter;
import org.apache.gravitino.catalog.jdbc.converter.SqliteExceptionConverter;
import org.apache.gravitino.catalog.jdbc.converter.SqliteTypeConverter;
import org.apache.gravitino.catalog.jdbc.operation.SqliteDatabaseOperations;
import org.apache.gravitino.catalog.jdbc.operation.SqliteTableOperations;
import org.apache.gravitino.exceptions.GravitinoRuntimeException;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;

public class TestJdbcCatalogOperations {

@Test
public void testTestConnection() {
JdbcCatalogOperations catalogOperations =
new JdbcCatalogOperations(
new SqliteExceptionConverter(),
new SqliteTypeConverter(),
new SqliteDatabaseOperations("/illegal/path"),
new SqliteTableOperations(),
new SqliteColumnDefaultValueConverter());
Assertions.assertThrows(
GravitinoRuntimeException.class,
() ->
catalogOperations.testConnection(
NameIdentifier.of("metalake", "catalog"),
Catalog.Type.RELATIONAL,
"sqlite",
"comment",
ImmutableMap.of()));
}
}
Loading
Loading