This is an automated email from the ASF dual-hosted git repository.
vinish pushed a commit to branch 590-CatalogSync
in repository https://gitbox.apache.org/repos/asf/incubator-xtable.git
The following commit(s) were added to refs/heads/590-CatalogSync by this push:
new d4e1c38e Remove ExternalCatalogConfigFactory and move logic to
CatalogConversionFactory
d4e1c38e is described below
commit d4e1c38e7f722d094dfc9a35e38c0c5ec3abac9c
Author: Vinish Reddy <[email protected]>
AuthorDate: Tue Dec 31 11:50:52 2024 -0800
Remove ExternalCatalogConfigFactory and move logic to
CatalogConversionFactory
---
.../xtable/catalog/CatalogConversionFactory.java | 29 ++++++++++
.../catalog/ExternalCatalogConfigFactory.java | 64 ----------------------
.../catalog/TestCatalogConversionFactory.java | 40 +++++++++++++-
.../catalog/TestExternalCatalogConfigFactory.java | 57 -------------------
.../apache/xtable/utilities/RunCatalogSync.java | 17 ------
5 files changed, 67 insertions(+), 140 deletions(-)
diff --git
a/xtable-core/src/main/java/org/apache/xtable/catalog/CatalogConversionFactory.java
b/xtable-core/src/main/java/org/apache/xtable/catalog/CatalogConversionFactory.java
index f453a0a9..add95c21 100644
---
a/xtable-core/src/main/java/org/apache/xtable/catalog/CatalogConversionFactory.java
+++
b/xtable-core/src/main/java/org/apache/xtable/catalog/CatalogConversionFactory.java
@@ -18,12 +18,17 @@
package org.apache.xtable.catalog;
+import java.util.ServiceLoader;
+import java.util.function.Function;
+
import lombok.AccessLevel;
import lombok.NoArgsConstructor;
+import org.apache.commons.lang3.StringUtils;
import org.apache.hadoop.conf.Configuration;
import org.apache.xtable.conversion.ExternalCatalogConfig;
+import org.apache.xtable.exception.NotSupportedException;
import org.apache.xtable.reflection.ReflectionUtils;
import org.apache.xtable.spi.extractor.CatalogConversionSource;
import org.apache.xtable.spi.sync.CatalogSyncClient;
@@ -45,6 +50,12 @@ public class CatalogConversionFactory {
*/
public static CatalogConversionSource createCatalogConversionSource(
ExternalCatalogConfig sourceCatalogConfig, Configuration configuration) {
+ if (!StringUtils.isEmpty(sourceCatalogConfig.getCatalogType())) {
+ return findInstance(
+ CatalogConversionSource.class,
+ sourceCatalogConfig.getCatalogType(),
+ CatalogConversionSource::getCatalogType);
+ }
return ReflectionUtils.createInstanceOfClass(
sourceCatalogConfig.getCatalogConversionSourceImpl(),
sourceCatalogConfig, configuration);
}
@@ -58,10 +69,28 @@ public class CatalogConversionFactory {
*/
public <TABLE> CatalogSyncClient<TABLE> createCatalogSyncClient(
ExternalCatalogConfig targetCatalogConfig, String tableFormat,
Configuration configuration) {
+ if (!StringUtils.isEmpty(targetCatalogConfig.getCatalogType())) {
+ return findInstance(
+ CatalogSyncClient.class,
+ targetCatalogConfig.getCatalogType(),
+ CatalogSyncClient::getCatalogType);
+ }
return ReflectionUtils.createInstanceOfClass(
targetCatalogConfig.getCatalogSyncClientImpl(),
targetCatalogConfig,
tableFormat,
configuration);
}
+
+ private static <T> T findInstance(
+ Class<T> serviceClass, String catalogType, Function<T, String>
catalogTypeExtractor) {
+ ServiceLoader<T> loader = ServiceLoader.load(serviceClass);
+ for (T instance : loader) {
+ String instanceCatalogType = catalogTypeExtractor.apply(instance);
+ if (catalogType.equals(instanceCatalogType)) {
+ return instance;
+ }
+ }
+ throw new NotSupportedException("catalogType is not yet supported: " +
catalogType);
+ }
}
diff --git
a/xtable-core/src/main/java/org/apache/xtable/catalog/ExternalCatalogConfigFactory.java
b/xtable-core/src/main/java/org/apache/xtable/catalog/ExternalCatalogConfigFactory.java
deleted file mode 100644
index 5a72463d..00000000
---
a/xtable-core/src/main/java/org/apache/xtable/catalog/ExternalCatalogConfigFactory.java
+++ /dev/null
@@ -1,64 +0,0 @@
-/*
- * 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.xtable.catalog;
-
-import java.util.Map;
-import java.util.ServiceLoader;
-import java.util.function.Function;
-
-import org.apache.xtable.conversion.ExternalCatalogConfig;
-import org.apache.xtable.exception.NotSupportedException;
-import org.apache.xtable.spi.extractor.CatalogConversionSource;
-import org.apache.xtable.spi.sync.CatalogSyncClient;
-
-/** A factory class which returns {@link ExternalCatalogConfig} based on
catalogType. */
-public class ExternalCatalogConfigFactory {
-
- public static ExternalCatalogConfig fromCatalogType(
- String catalogType, String catalogId, Map<String, String> properties) {
- String catalogSyncClientImpl =
- findInstance(CatalogSyncClient.class, catalogType,
CatalogSyncClient::getCatalogType)
- .getClass()
- .getName();
- String catalogConversionSourceImpl =
- findInstance(
- CatalogConversionSource.class, catalogType,
CatalogConversionSource::getCatalogType)
- .getClass()
- .getName();
- return ExternalCatalogConfig.builder()
- .catalogType(catalogType)
- .catalogSyncClientImpl(catalogSyncClientImpl)
- .catalogConversionSourceImpl(catalogConversionSourceImpl)
- .catalogId(catalogId)
- .catalogProperties(properties)
- .build();
- }
-
- private static <T> T findInstance(
- Class<T> serviceClass, String catalogType, Function<T, String>
catalogTypeExtractor) {
- ServiceLoader<T> loader = ServiceLoader.load(serviceClass);
- for (T instance : loader) {
- String instanceCatalogType = catalogTypeExtractor.apply(instance);
- if (catalogType.equals(instanceCatalogType)) {
- return instance;
- }
- }
- throw new NotSupportedException("catalogType is not yet supported: " +
catalogType);
- }
-}
diff --git
a/xtable-core/src/test/java/org/apache/xtable/catalog/TestCatalogConversionFactory.java
b/xtable-core/src/test/java/org/apache/xtable/catalog/TestCatalogConversionFactory.java
index d0b10d2f..1d05666b 100644
---
a/xtable-core/src/test/java/org/apache/xtable/catalog/TestCatalogConversionFactory.java
+++
b/xtable-core/src/test/java/org/apache/xtable/catalog/TestCatalogConversionFactory.java
@@ -30,13 +30,14 @@ import org.apache.xtable.conversion.TargetCatalogConfig;
import org.apache.xtable.model.catalog.ThreePartHierarchicalTableIdentifier;
import org.apache.xtable.spi.extractor.CatalogConversionSource;
import org.apache.xtable.spi.sync.CatalogSyncClient;
+import org.apache.xtable.testutil.ITTestUtils;
import org.apache.xtable.testutil.ITTestUtils.TestCatalogConversionSourceImpl;
import org.apache.xtable.testutil.ITTestUtils.TestCatalogSyncImpl;
class TestCatalogConversionFactory {
@Test
- void createSourceForConfig() {
+ void createCatalogConversionSource() {
ExternalCatalogConfig sourceCatalog =
ExternalCatalogConfig.builder()
.catalogId("catalogId")
@@ -51,7 +52,22 @@ class TestCatalogConversionFactory {
}
@Test
- void createForCatalog() {
+ void createCatalogConversionSourceForCatalogType() {
+ ExternalCatalogConfig sourceCatalog =
+ ExternalCatalogConfig.builder()
+ .catalogId("catalogId")
+ .catalogType(ITTestUtils.TEST_CATALOG_TYPE)
+ .catalogProperties(Collections.emptyMap())
+ .build();
+ CatalogConversionSource catalogConversionSource =
+ CatalogConversionFactory.createCatalogConversionSource(sourceCatalog,
new Configuration());
+ assertEquals(
+ catalogConversionSource.getClass().getName(),
+ TestCatalogConversionSourceImpl.class.getName());
+ }
+
+ @Test
+ void createCatalogSyncClient() {
TargetCatalogConfig targetCatalogConfig =
TargetCatalogConfig.builder()
.catalogConfig(
@@ -69,4 +85,24 @@ class TestCatalogConversionFactory {
targetCatalogConfig.getCatalogConfig(), "TABLE_FORMAT", new
Configuration());
assertEquals(catalogSyncClient.getClass().getName(),
TestCatalogSyncImpl.class.getName());
}
+
+ @Test
+ void createCatalogSyncClientForCatalogType() {
+ TargetCatalogConfig targetCatalogConfig =
+ TargetCatalogConfig.builder()
+ .catalogConfig(
+ ExternalCatalogConfig.builder()
+ .catalogId("catalogId")
+ .catalogType(ITTestUtils.TEST_CATALOG_TYPE)
+ .catalogProperties(Collections.emptyMap())
+ .build())
+ .catalogTableIdentifier(
+ new ThreePartHierarchicalTableIdentifier("target-database",
"target-tableName"))
+ .build();
+ CatalogSyncClient catalogSyncClient =
+ CatalogConversionFactory.getInstance()
+ .createCatalogSyncClient(
+ targetCatalogConfig.getCatalogConfig(), "TABLE_FORMAT", new
Configuration());
+ assertEquals(catalogSyncClient.getClass().getName(),
TestCatalogSyncImpl.class.getName());
+ }
}
diff --git
a/xtable-core/src/test/java/org/apache/xtable/catalog/TestExternalCatalogConfigFactory.java
b/xtable-core/src/test/java/org/apache/xtable/catalog/TestExternalCatalogConfigFactory.java
deleted file mode 100644
index d159b51b..00000000
---
a/xtable-core/src/test/java/org/apache/xtable/catalog/TestExternalCatalogConfigFactory.java
+++ /dev/null
@@ -1,57 +0,0 @@
-/*
- * 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.xtable.catalog;
-
-import static org.apache.xtable.testutil.ITTestUtils.TEST_CATALOG_TYPE;
-import static org.junit.jupiter.api.Assertions.*;
-
-import java.util.Collections;
-import java.util.UUID;
-
-import org.junit.jupiter.api.Assertions;
-import org.junit.jupiter.api.Test;
-
-import org.apache.xtable.conversion.ExternalCatalogConfig;
-import org.apache.xtable.exception.NotSupportedException;
-import org.apache.xtable.testutil.ITTestUtils;
-
-class TestExternalCatalogConfigFactory {
-
- @Test
- void testFromCatalogType() {
- ExternalCatalogConfig externalCatalogConfig =
- ExternalCatalogConfigFactory.fromCatalogType(
- TEST_CATALOG_TYPE, UUID.randomUUID().toString(),
Collections.emptyMap());
- Assertions.assertEquals(
- ITTestUtils.TestCatalogSyncImpl.class.getName(),
- externalCatalogConfig.getCatalogSyncClientImpl());
- Assertions.assertEquals(
- ITTestUtils.TestCatalogConversionSourceImpl.class.getName(),
- externalCatalogConfig.getCatalogConversionSourceImpl());
- }
-
- @Test
- void testFromCatalogTypeNotFound() {
- Assertions.assertThrows(
- NotSupportedException.class,
- () ->
- ExternalCatalogConfigFactory.fromCatalogType(
- "invalid", UUID.randomUUID().toString(),
Collections.emptyMap()));
- }
-}
diff --git
a/xtable-utilities/src/main/java/org/apache/xtable/utilities/RunCatalogSync.java
b/xtable-utilities/src/main/java/org/apache/xtable/utilities/RunCatalogSync.java
index 1536ab9c..60a62cd9 100644
---
a/xtable-utilities/src/main/java/org/apache/xtable/utilities/RunCatalogSync.java
+++
b/xtable-utilities/src/main/java/org/apache/xtable/utilities/RunCatalogSync.java
@@ -46,14 +46,12 @@ import org.apache.commons.cli.DefaultParser;
import org.apache.commons.cli.HelpFormatter;
import org.apache.commons.cli.Options;
import org.apache.commons.cli.ParseException;
-import org.apache.commons.lang3.StringUtils;
import org.apache.hadoop.conf.Configuration;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.dataformat.yaml.YAMLFactory;
import org.apache.xtable.catalog.CatalogConversionFactory;
-import org.apache.xtable.catalog.ExternalCatalogConfigFactory;
import org.apache.xtable.conversion.ConversionConfig;
import org.apache.xtable.conversion.ConversionController;
import org.apache.xtable.conversion.ConversionSourceProvider;
@@ -141,7 +139,6 @@ public class RunCatalogSync {
Map<String, ExternalCatalogConfig> catalogsById =
datasetConfig.getTargetCatalogs().stream()
- .map(RunCatalogSync::populateCatalogImplementations)
.collect(Collectors.toMap(ExternalCatalogConfig::getCatalogId,
Function.identity()));
Optional<CatalogConversionSource> catalogConversionSource =
getCatalogConversionSource(datasetConfig.getSourceCatalog(),
hadoopConf);
@@ -276,20 +273,6 @@ public class RunCatalogSync {
throw new IllegalArgumentException("Invalid tableIdentifier configuration
provided");
}
- /**
- * If user provides catalogType, we try to populate the implementation class
if it exists in the
- * class path.
- */
- static ExternalCatalogConfig
populateCatalogImplementations(ExternalCatalogConfig catalogConfig) {
- if (!StringUtils.isEmpty(catalogConfig.getCatalogType())) {
- return ExternalCatalogConfigFactory.fromCatalogType(
- catalogConfig.getCatalogType(),
- catalogConfig.getCatalogId(),
- catalogConfig.getCatalogProperties());
- }
- return catalogConfig;
- }
-
@Value
@Builder
@Jacksonized
