Copilot commented on code in PR #13212:
URL: https://github.com/apache/gravitino/pull/13212#discussion_r4025429063
##########
trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/catalog/DefaultCatalogConnectorFactory.java:
##########
@@ -81,9 +84,43 @@ public DefaultCatalogConnectorFactory(GravitinoConfig
config) {
catalogBuilders.put(
TRINO_CLUSTER_CONNECTOR_PROVIDER_NAME,
new CatalogConnectorContext.Builder(new
TrinoClusterConnectorAdapter()));
+ registerAdapterProviders(config);
LOG.info("Start the DefaultCatalogConnectorFactory");
}
+ /**
+ * Adds the adapters contributed through {@link
CatalogConnectorAdapterProvider}. A provider for a
+ * catalog provider name that is already registered is ignored.
+ */
+ private void registerAdapterProviders(GravitinoConfig config) {
+ Iterator<CatalogConnectorAdapterProvider> iterator =
+ ServiceLoader.load(
+ CatalogConnectorAdapterProvider.class,
+ DefaultCatalogConnectorFactory.class.getClassLoader())
+ .iterator();
+ while (true) {
+ try {
+ if (!iterator.hasNext()) {
+ return;
+ }
+ CatalogConnectorAdapterProvider provider = iterator.next();
+ String providerName = provider.provider();
+ if (catalogBuilders.containsKey(providerName)) {
+ LOG.warn(
+ "Ignore catalog connector adapter provider %s for %s: already
registered.",
+ provider.getClass().getName(), providerName);
+ continue;
+ }
+ catalogBuilders.put(
+ providerName, new
CatalogConnectorContext.Builder(provider.createAdapter(config)));
+ LOG.info("Registered catalog connector adapter for %s", providerName);
+ } catch (ServiceConfigurationError e) {
Review Comment:
Provider methods run inside this try, not just ServiceLoader instantiation.
An adapter compiled against a different Trino SPI can throw
`NoClassDefFoundError` or `AbstractMethodError` from
`provider()`/`createAdapter()`; those are `LinkageError`s, not
`ServiceConfigurationError`, so they escape and fail connector initialization
instead of skipping the broken provider as intended. Catch `LinkageError` as
well, as the analogous Flink ServiceLoader path does in
`flink-connector/flink-common/src/main/java/org/apache/gravitino/flink/connector/store/GravitinoCatalogStore.java:199-206`.
--
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]
