liugddx commented on code in PR #10657:
URL: https://github.com/apache/seatunnel/pull/10657#discussion_r2998518497
##########
seatunnel-api/src/main/java/org/apache/seatunnel/api/metadata/MetaDataProviderFactory.java:
##########
@@ -38,30 +37,30 @@
* </ul>
*/
@Slf4j
-public final class DataSourceProviderFactory {
+public final class MetaDataProviderFactory {
/**
- * Finds a {@link DataSourceProvider} by its kind identifier.
+ * Finds a {@link MetaDataProvider} by its kind identifier.
*
* @param kind the kind identifier of the provider to find
* @return the provider
- * @throws DataSourceProviderException if provider is not found or
multiple providers with the
+ * @throws MetaDataProviderException if provider is not found or multiple
providers with the
* same kind are found
*/
- public static DataSourceProvider getProvider(String kind) {
- List<DataSourceProvider> providers = loadProviders();
+ public static MetaDataProvider getProvider(String kind) {
+ List<MetaDataProvider> providers = loadProviders();
- DataSourceProvider matchedProvider = null;
+ MetaDataProvider matchedProvider = null;
List<String> matchedKinds = new ArrayList<>();
- for (DataSourceProvider provider : providers) {
+ for (MetaDataProvider provider : providers) {
if (provider.kind().equalsIgnoreCase(kind)) {
if (matchedProvider != null) {
log.error(
"Multiple DataSourceProvider implementations found
for kind '{}': {}",
kind,
matchedKinds);
- throw new DataSourceProviderException(
+ throw new MetaDataProviderException(
String.format(
"Multiple DataSourceProvider
implementations found for kind '%s'.\n\n"
Review Comment:
[Must Fix] Two issues here:
1. Rename "DataSourceProvider" → "MetaDataProvider" in this exception
message.
2. Bug: When the second duplicate provider is found, the exception is thrown
BEFORE
`matchedKinds.add(provider.getClass().getName())` at line 70. So the
error message
"Ambiguous provider classes" only lists ONE class instead of both.
Fix: move `matchedKinds.add(...)` before the `if (matchedProvider !=
null)` check:
matchedKinds.add(provider.getClass().getName());
if (matchedProvider != null) {
throw new MetaDataProviderException(...);
}
matchedProvider = provider;
##########
seatunnel-api/src/main/java/org/apache/seatunnel/api/metadata/MetaDataProviderFactory.java:
##########
@@ -38,30 +37,30 @@
* </ul>
*/
@Slf4j
-public final class DataSourceProviderFactory {
+public final class MetaDataProviderFactory {
/**
- * Finds a {@link DataSourceProvider} by its kind identifier.
+ * Finds a {@link MetaDataProvider} by its kind identifier.
*
* @param kind the kind identifier of the provider to find
* @return the provider
- * @throws DataSourceProviderException if provider is not found or
multiple providers with the
+ * @throws MetaDataProviderException if provider is not found or multiple
providers with the
* same kind are found
*/
- public static DataSourceProvider getProvider(String kind) {
- List<DataSourceProvider> providers = loadProviders();
+ public static MetaDataProvider getProvider(String kind) {
+ List<MetaDataProvider> providers = loadProviders();
- DataSourceProvider matchedProvider = null;
+ MetaDataProvider matchedProvider = null;
List<String> matchedKinds = new ArrayList<>();
- for (DataSourceProvider provider : providers) {
+ for (MetaDataProvider provider : providers) {
if (provider.kind().equalsIgnoreCase(kind)) {
if (matchedProvider != null) {
log.error(
"Multiple DataSourceProvider implementations found
for kind '{}': {}",
Review Comment:
[Must Fix] Rename to "Multiple MetaDataProvider implementations found for
kind '{}': {}"
The SPI has been renamed from DataSource to Metadata, but this log message
still uses the old name.
##########
seatunnel-api/src/main/java/org/apache/seatunnel/api/metadata/MetaDataProviderFactory.java:
##########
@@ -74,11 +73,11 @@ public static DataSourceProvider getProvider(String kind) {
if (matchedProvider == null) {
List<String> availableKinds = new ArrayList<>();
- for (DataSourceProvider provider : providers) {
+ for (MetaDataProvider provider : providers) {
availableKinds.add(provider.kind());
}
log.debug("No DataSourceProvider found for kind: {}", kind);
Review Comment:
[Must Fix] Rename to "No MetaDataProvider found for kind: {}"
##########
seatunnel-api/src/main/java/org/apache/seatunnel/api/metadata/exception/MetaDataProviderException.java:
##########
@@ -15,29 +15,29 @@
* limitations under the License.
*/
-package org.apache.seatunnel.api.datasource.exception;
+package org.apache.seatunnel.api.metadata.exception;
import org.apache.seatunnel.api.common.SeaTunnelAPIErrorCode;
import org.apache.seatunnel.common.exception.SeaTunnelRuntimeException;
/** A DataSourceProvider-related, runtime exception. */
Review Comment:
[Must Fix] Rename Javadoc to "A MetaDataProvider-related, runtime exception."
##########
seatunnel-api/src/main/java/org/apache/seatunnel/api/metadata/MetaDataProviderManager.java:
##########
Review Comment:
[Must Fix] Rename to "MetaDataProvider closed"
##########
seatunnel-api/src/main/java/org/apache/seatunnel/api/metadata/MetaDataProviderFactory.java:
##########
@@ -118,15 +117,15 @@ private static List<DataSourceProvider> loadProviders() {
"Loaded {} DataSourceProvider: {}",
providers.size(),
providers.stream()
- .map(DataSourceProvider::kind)
+ .map(MetaDataProvider::kind)
.reduce((a, b) -> a + ", " + b)
.orElse(""));
}
return providers;
} catch (ServiceConfigurationError e) {
log.error("Could not load service provider for
DataSourceProvider.", e);
- throw new DataSourceProviderException(
+ throw new MetaDataProviderException(
Review Comment:
[Must Fix] Rename both occurrences to "Could not load service provider for
MetaDataProvider."
##########
seatunnel-api/src/main/java/org/apache/seatunnel/api/metadata/MetaDataOptions.java:
##########
Review Comment:
[Must Fix] Rename "DataSource Center" to "Metadata Center" in the
description.
##########
seatunnel-api/src/main/java/org/apache/seatunnel/api/metadata/MetaDataConfig.java:
##########
@@ -31,16 +31,16 @@
* the properties map.
*/
@Data
-public class DataSourceConfig implements Serializable {
+public class MetaDataConfig implements Serializable {
/** Whether to enable DataSource Center. */
Review Comment:
[Must Fix] Rename to "Whether to enable Metadata Center."
##########
seatunnel-api/src/main/java/org/apache/seatunnel/api/metadata/MetaDataProviderFactory.java:
##########
@@ -74,11 +73,11 @@ public static DataSourceProvider getProvider(String kind) {
if (matchedProvider == null) {
List<String> availableKinds = new ArrayList<>();
- for (DataSourceProvider provider : providers) {
+ for (MetaDataProvider provider : providers) {
availableKinds.add(provider.kind());
}
log.debug("No DataSourceProvider found for kind: {}", kind);
- throw new DataSourceProviderException(
+ throw new MetaDataProviderException(
String.format(
Review Comment:
[Must Fix] Rename to "No MetaDataProvider found for kind '%s'..."
##########
seatunnel-api/src/main/java/org/apache/seatunnel/api/metadata/MetaDataProviderFactory.java:
##########
Review Comment:
[Must Fix] Rename to "No MetaDataProvider implementations found"
##########
seatunnel-api/src/main/java/org/apache/seatunnel/api/metadata/MetaDataProviderManager.java:
##########
@@ -45,13 +43,13 @@
* Utility class for resolving data source configurations from DataSource
Center.
*
* <p>This utility provides methods to merge connection configurations
retrieved from external
- * metadata services (via {@link DataSourceProvider}) into SeaTunnel connector
configurations.
+ * metadata services (via {@link MetaDataProvider}) into SeaTunnel connector
configurations.
*/
@Slf4j
-public final class DataSourceConfigResolver {
+public final class MetaDataProviderManager {
/** Cache for initialized DataSourceProvider instance. */
- private static volatile DataSourceProvider cachedProvider = null;
+ private static volatile MetaDataProvider cachedProvider = null;
/**
* Resolves and merges data source configurations for a SeaTunnel job
config.
Review Comment:
[Must Fix] Rename Javadoc to "Cache for initialized MetaDataProvider
instance."
##########
seatunnel-api/src/main/java/org/apache/seatunnel/api/metadata/MetaDataProviderManager.java:
##########
@@ -108,20 +106,32 @@ public static Config resolveDataSourceConfigs(
return ConfigFactory.parseMap(resultMap);
}
+ public static Optional<TableSchema> resolveTableSchema(
+ String metaDataTableId, MetaDataConfig dataSourceConfig) {
+ if (dataSourceConfig == null || !dataSourceConfig.isEnabled()) {
+ return Optional.empty();
+ }
+ MetaDataProvider provider =
+ getOrCreateProvider(
+ dataSourceConfig.getKind(),
+
ConfigFactory.parseMap(dataSourceConfig.getProperties()));
+ return provider.tableSchema(metaDataTableId);
+ }
+
/**
* Gets or creates an initialized DataSourceProvider instance with lazy
loading caching.
*
* @param kind the provider kind (e.g., "gravitino", "datahub")
* @param config the configuration for the provider
* @return initialized DataSourceProvider instance
*/
- private static DataSourceProvider getOrCreateProvider(String kind, Config
config) {
- DataSourceProvider provider = cachedProvider;
+ private static MetaDataProvider getOrCreateProvider(String kind, Config
config) {
Review Comment:
[Should Fix] This cache only checks `cachedProvider == null` but does NOT
validate whether
the cached provider's `kind` matches the requested `kind`. If called with
different kinds
in different contexts, it will silently return the wrong provider.
Suggest adding a kind check:
if (provider != null && !provider.kind().equalsIgnoreCase(kind)) {
log.warn("Cached provider kind '{}' does not match requested kind
'{}', recreating",
provider.kind(), kind);
provider.close();
provider = null;
cachedProvider = null;
}
##########
seatunnel-api/src/main/java/org/apache/seatunnel/api/metadata/MetaDataProviderFactory.java:
##########
@@ -118,15 +117,15 @@ private static List<DataSourceProvider> loadProviders() {
"Loaded {} DataSourceProvider: {}",
Review Comment:
[Must Fix] Rename to "Loaded {} MetaDataProvider: {}"
##########
seatunnel-api/src/main/java/org/apache/seatunnel/api/metadata/MetaDataProviderManager.java:
##########
@@ -108,20 +106,32 @@ public static Config resolveDataSourceConfigs(
return ConfigFactory.parseMap(resultMap);
}
+ public static Optional<TableSchema> resolveTableSchema(
+ String metaDataTableId, MetaDataConfig dataSourceConfig) {
+ if (dataSourceConfig == null || !dataSourceConfig.isEnabled()) {
+ return Optional.empty();
+ }
+ MetaDataProvider provider =
+ getOrCreateProvider(
+ dataSourceConfig.getKind(),
+
ConfigFactory.parseMap(dataSourceConfig.getProperties()));
+ return provider.tableSchema(metaDataTableId);
+ }
+
/**
* Gets or creates an initialized DataSourceProvider instance with lazy
loading caching.
*
* @param kind the provider kind (e.g., "gravitino", "datahub")
* @param config the configuration for the provider
* @return initialized DataSourceProvider instance
*/
- private static DataSourceProvider getOrCreateProvider(String kind, Config
config) {
- DataSourceProvider provider = cachedProvider;
+ private static MetaDataProvider getOrCreateProvider(String kind, Config
config) {
+ MetaDataProvider provider = cachedProvider;
if (provider == null) {
- synchronized (DataSourceConfigResolver.class) {
+ synchronized (MetaDataProviderManager.class) {
provider = cachedProvider;
if (provider == null) {
- provider = DataSourceProviderFactory.getProvider(kind);
+ provider = MetaDataProviderFactory.getProvider(kind);
provider.init(config);
cachedProvider = provider;
log.info("Created and cached new DataSourceProvider: {}",
kind);
Review Comment:
[Must Fix] Rename to "Created and cached new MetaDataProvider: {}"
##########
seatunnel-api/src/main/java/org/apache/seatunnel/api/metadata/MetaDataProviderManager.java:
##########
@@ -328,7 +340,7 @@ private static Config mergeConfig(
* <p>This method is idempotent and can be safely called multiple times.
*/
public static void closeProviders() {
- DataSourceProvider provider = cachedProvider;
+ MetaDataProvider provider = cachedProvider;
if (provider != null) {
try {
log.info("Closing cached DataSourceProvider");
Review Comment:
[Must Fix] Rename to "Closing cached MetaDataProvider"
##########
seatunnel-api/src/main/java/org/apache/seatunnel/api/metadata/MetaDataConfig.java:
##########
Review Comment:
[Must Fix] Rename Javadoc to "Configuration for Metadata Center which
manages external metadata providers."
--
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]
