yuqi1129 commented on code in PR #9735:
URL: https://github.com/apache/gravitino/pull/9735#discussion_r2731769148
##########
trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/catalog/CatalogConnectorManager.java:
##########
@@ -369,24 +374,32 @@ public Set<String> getUsedMetalakes() {
* @param connectorName the name of the connector
* @param config the Gravitino configuration
* @param context the Trino connector context
- * @return the created connector
+ * @return the created catalog connector context
*/
- public Connector createConnector(
+ public CatalogConnectorContext createCatalogConnectorContext(
String connectorName, GravitinoConfig config, ConnectorContext context) {
try {
String catalogConfig = config.getCatalogConfig();
GravitinoCatalog catalog = GravitinoCatalog.fromJson(catalogConfig);
+ if (this.config.singleMetalakeMode()
+ && !StringUtils.isNotBlank(targetMetalake)
Review Comment:
!StringUtils.isNotBlank, you may need to remove `!` here.
##########
trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/GravitinoConnector.java:
##########
@@ -86,14 +87,15 @@ public ConnectorMetadata getMetadata(
ConnectorMetadata internalMetadata =
internalConnector.getMetadata(session,
gravitinoTransactionHandle.getInternalHandle());
Preconditions.checkArgument(internalMetadata != null, "Internal metadata
must not be null");
+ return createGravitinoMetadata(
+ connectorMetadata, catalogConnectorContext.getMetadataAdapter(),
internalMetadata);
+ }
- GravitinoMetalake metalake = catalogConnectorContext.getMetalake();
-
- CatalogConnectorMetadata catalogConnectorMetadata =
- new CatalogConnectorMetadata(metalake, catalogIdentifier);
-
- return new GravitinoMetadata(
- catalogConnectorMetadata,
catalogConnectorContext.getMetadataAdapter(), internalMetadata);
+ protected GravitinoMetadata createGravitinoMetadata(
+ CatalogConnectorMetadata catalogConnectorMetadata,
+ CatalogConnectorMetadataAdapter metadataAdapter,
+ ConnectorMetadata internalMetadata) {
+ throw new RuntimeException("Should be overridden in subclass");
Review Comment:
This shoud be resolved.
##########
trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/GravitinoConnectorFactory.java:
##########
@@ -117,13 +137,70 @@ public Connector create(
}
GravitinoStoredProcedureFactory gravitinoStoredProcedureFactory =
new GravitinoStoredProcedureFactory(catalogConnectorManager,
metalake);
- return new GravitinoSystemConnector(gravitinoStoredProcedureFactory);
+ return createSystemConnector(gravitinoStoredProcedureFactory);
}
}
- @VisibleForTesting
- Supplier<GravitinoAdminClient> clientProvider() {
- return () -> null;
+ protected GravitinoConnector createConnector(CatalogConnectorContext
connectorContext) {
+ return new GravitinoConnector(connectorContext);
+ }
+
+ protected GravitinoSystemConnector createSystemConnector(
+ GravitinoStoredProcedureFactory storedProcedureFactory) {
+ return new GravitinoSystemConnector(storedProcedureFactory);
+ }
+
+ protected String getTrinoCatalogName(String metalakeName, String
catalogName) {
+ return "\"" + metalakeName + "." + catalogName + "\"";
+ }
+
+ private void checkTrinoSpiVersion(ConnectorContext context, GravitinoConfig
config) {
+ String spiVersion = context.getSpiVersion();
+
+ trinoVersion = Integer.parseInt(spiVersion);
+ if (trinoVersion < getMinSupportTrinoSpiVersion()
+ || trinoVersion > getMaxSupportTrinoSpiVersion()) {
+ Boolean skipTrinoVersionValidation =
config.isSkipTrinoVersionValidation();
Review Comment:
What I mean here if `isSkipTrinoVersionValidation` is true, should we just
skip calling the method `checkTrinoSpiVersion`?
--
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]
