FANNG1 commented on code in PR #5087:
URL: https://github.com/apache/gravitino/pull/5087#discussion_r1796388297
##########
catalogs/catalog-lakehouse-paimon/build.gradle.kts:
##########
@@ -122,12 +122,14 @@ dependencies {
testImplementation(libs.junit.jupiter.api)
testImplementation(libs.mysql.driver)
testImplementation(libs.postgresql.driver)
+ testImplementation(libs.h2db)
testImplementation(libs.bundles.log4j)
testImplementation(libs.junit.jupiter.params)
testImplementation(libs.paimon.s3)
testImplementation(libs.paimon.spark)
testImplementation(libs.testcontainers)
testImplementation(libs.testcontainers.localstack)
+ testImplementation(libs.testcontainers.mysql)
Review Comment:
why adding this?
##########
catalogs/catalog-lakehouse-paimon/src/main/java/org/apache/gravitino/catalog/lakehouse/paimon/utils/CatalogUtils.java:
##########
@@ -121,6 +123,17 @@ private static void checkPaimonConfig(PaimonConfig
paimonConfig) {
Preconditions.checkArgument(
StringUtils.isNotBlank(uri), "Paimon Catalog uri can not be null or
empty.");
}
+
+ if (PaimonCatalogBackend.JDBC.name().equalsIgnoreCase(metastore)) {
+ String jdbcUser = paimonConfig.get(CATALOG_JDBC_USER);
Review Comment:
you could add validate check in ConfigEntry like
```java
public static final ConfigEntry<String> JDBC_USER =
new ConfigBuilder(IcebergConstants.GRAVITINO_JDBC_USER)
.doc("The username of the Jdbc connection")
.version(ConfigConstants.VERSION_0_2_0)
.stringConf()
.checkValue(StringUtils::isNotBlank,
ConfigConstants.NOT_BLANK_ERROR_MSG)
.create();
```
##########
catalogs/catalog-lakehouse-paimon/src/main/java/org/apache/gravitino/catalog/lakehouse/paimon/PaimonCatalogPropertiesMetadata.java:
##########
@@ -46,14 +46,26 @@ public class PaimonCatalogPropertiesMetadata extends
BaseCatalogPropertiesMetada
public static final String PAIMON_METASTORE = "metastore";
public static final String WAREHOUSE = "warehouse";
public static final String URI = "uri";
+ public static final String JDBC_USER = "jdbc.user";
Review Comment:
could you use `jdbc-user` like Iceberg?
##########
catalogs/catalog-lakehouse-paimon/src/test/java/org/apache/gravitino/catalog/lakehouse/paimon/integration/test/CatalogPaimonFileSystemIT.java:
##########
@@ -48,4 +55,22 @@ protected Map<String, String> initPaimonCatalogProperties() {
return catalogProperties;
}
+
+ @Test
+ void testPaimonSchemaProperties() throws Catalog.DatabaseNotExistException {
Review Comment:
no need to add this test for schema properties, since it's empty.
--
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]
