This is an automated email from the ASF dual-hosted git repository.
jshao pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new 441e6ed5f [#5756] Bug Fix : Warehouse parameter systematically
required (#5923)
441e6ed5f is described below
commit 441e6ed5f3d94a0e2b6652600457d54738d61875
Author: fsalhi2 <[email protected]>
AuthorDate: Mon Dec 23 04:03:41 2024 +0100
[#5756] Bug Fix : Warehouse parameter systematically required (#5923)
### What changes were proposed in this pull request?
Removed the systematic validations in Iceberg Config and modified the
instantiation of the warehouse attribute and uri attribute in the
IcebergCatalogWrapper to conform with the new possibility (REST).
### Why are the changes needed?
The bug was blocking the creation of a rest catalog.
Fix: [#5756](https://github.com/apache/gravitino/issues/5756)
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
./gradlew build && compileDistribution && assembleDistribution +
creation of the docker image there :
https://hub.docker.com/r/fsalhi2/gravitino
Started gravitino with such configs :
```
### Gravitino General Settings
gravitino.auxService.names = iceberg-rest
gravitino.iceberg-rest.classpath = iceberg-rest-server/libs,
iceberg-rest-server/conf
### HTTP Server
gravitino.iceberg-rest.host = 0.0.0.0
gravitino.iceberg-rest.httpPort = 9001
### Storage
gravitino.iceberg-rest.io-impl = org.apache.iceberg.aws.s3.S3FileIO
gravitino.iceberg-rest.s3-access-key-id = XXXXX
gravitino.iceberg-rest.s3-secret-access-key = XXXXXX
gravitino.iceberg-rest.s3-path-style-access = true
gravitino.iceberg-rest.s3-endpoint = http://minio:9000/
gravitino.iceberg-rest.s3-region = us-east-1
### JDBC
gravitino.iceberg-rest.catalog-backend = jdbc
gravitino.iceberg-rest.uri = jdbc:mysql://mysql:3306/
gravitino.iceberg-rest.warehouse = s3://lake/catalog
gravitino.iceberg-rest.jdbc.user = root
gravitino.iceberg-rest.jdbc.password = XXXXXX
gravitino.iceberg-rest.jdbc-driver = com.mysql.cj.jdbc.Driver
```
Was able to create a catalog through Web UI and start working on the
scheme.
---
.../iceberg/IcebergCatalogPropertiesMetadata.java | 3 +-
.../lakehouse/iceberg/TestIcebergCatalog.java | 47 ++++++++++++++++++++++
.../gravitino/iceberg/common/IcebergConfig.java | 1 -
.../iceberg/common/ops/IcebergCatalogWrapper.java | 10 ++++-
4 files changed, 57 insertions(+), 4 deletions(-)
diff --git
a/catalogs/catalog-lakehouse-iceberg/src/main/java/org/apache/gravitino/catalog/lakehouse/iceberg/IcebergCatalogPropertiesMetadata.java
b/catalogs/catalog-lakehouse-iceberg/src/main/java/org/apache/gravitino/catalog/lakehouse/iceberg/IcebergCatalogPropertiesMetadata.java
index 6d61a6220..375edd600 100644
---
a/catalogs/catalog-lakehouse-iceberg/src/main/java/org/apache/gravitino/catalog/lakehouse/iceberg/IcebergCatalogPropertiesMetadata.java
+++
b/catalogs/catalog-lakehouse-iceberg/src/main/java/org/apache/gravitino/catalog/lakehouse/iceberg/IcebergCatalogPropertiesMetadata.java
@@ -74,10 +74,11 @@ public class IcebergCatalogPropertiesMetadata extends
BaseCatalogPropertiesMetad
false /* reserved */),
stringRequiredPropertyEntry(
URI, "Iceberg catalog uri config", false /* immutable */,
false /* hidden */),
- stringRequiredPropertyEntry(
+ stringOptionalPropertyEntry(
WAREHOUSE,
"Iceberg catalog warehouse config",
false /* immutable */,
+ null, /* defaultValue */
false /* hidden */),
stringOptionalPropertyEntry(
IcebergConstants.IO_IMPL,
diff --git
a/catalogs/catalog-lakehouse-iceberg/src/test/java/org/apache/gravitino/catalog/lakehouse/iceberg/TestIcebergCatalog.java
b/catalogs/catalog-lakehouse-iceberg/src/test/java/org/apache/gravitino/catalog/lakehouse/iceberg/TestIcebergCatalog.java
index 5c6571972..8ff70d398 100644
---
a/catalogs/catalog-lakehouse-iceberg/src/test/java/org/apache/gravitino/catalog/lakehouse/iceberg/TestIcebergCatalog.java
+++
b/catalogs/catalog-lakehouse-iceberg/src/test/java/org/apache/gravitino/catalog/lakehouse/iceberg/TestIcebergCatalog.java
@@ -146,4 +146,51 @@ public class TestIcebergCatalog {
throwable.getMessage().contains(IcebergCatalogPropertiesMetadata.CATALOG_BACKEND));
}
}
+
+ @Test
+ void testCatalogInstanciation() {
+ AuditInfo auditInfo =
+
AuditInfo.builder().withCreator("creator").withCreateTime(Instant.now()).build();
+
+ CatalogEntity entity =
+ CatalogEntity.builder()
+ .withId(1L)
+ .withName("catalog")
+ .withNamespace(Namespace.of("metalake"))
+ .withType(IcebergCatalog.Type.RELATIONAL)
+ .withProvider("iceberg")
+ .withAuditInfo(auditInfo)
+ .build();
+
+ Map<String, String> conf = Maps.newHashMap();
+
+ try (IcebergCatalogOperations ops = new IcebergCatalogOperations()) {
+ ops.initialize(conf, entity.toCatalogInfo(),
ICEBERG_PROPERTIES_METADATA);
+ Map<String, String> map1 = Maps.newHashMap();
+ map1.put(IcebergCatalogPropertiesMetadata.CATALOG_BACKEND, "test");
+ PropertiesMetadata metadata =
ICEBERG_PROPERTIES_METADATA.catalogPropertiesMetadata();
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () -> {
+ PropertiesMetadataHelpers.validatePropertyForCreate(metadata,
map1);
+ });
+
+ Map<String, String> map2 = Maps.newHashMap();
+ map2.put(IcebergCatalogPropertiesMetadata.CATALOG_BACKEND, "rest");
+ map2.put(IcebergCatalogPropertiesMetadata.URI, "127.0.0.1");
+ Assertions.assertDoesNotThrow(
+ () -> {
+ PropertiesMetadataHelpers.validatePropertyForCreate(metadata,
map2);
+ });
+
+ Map<String, String> map3 = Maps.newHashMap();
+ Throwable throwable =
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () ->
PropertiesMetadataHelpers.validatePropertyForCreate(metadata, map3));
+
+ Assertions.assertTrue(
+
throwable.getMessage().contains(IcebergCatalogPropertiesMetadata.CATALOG_BACKEND));
+ }
+ }
}
diff --git
a/iceberg/iceberg-common/src/main/java/org/apache/gravitino/iceberg/common/IcebergConfig.java
b/iceberg/iceberg-common/src/main/java/org/apache/gravitino/iceberg/common/IcebergConfig.java
index 2e7eb74e2..60a7491b8 100644
---
a/iceberg/iceberg-common/src/main/java/org/apache/gravitino/iceberg/common/IcebergConfig.java
+++
b/iceberg/iceberg-common/src/main/java/org/apache/gravitino/iceberg/common/IcebergConfig.java
@@ -65,7 +65,6 @@ public class IcebergConfig extends Config implements
OverwriteDefaultConfig {
.doc("Warehouse directory of catalog")
.version(ConfigConstants.VERSION_0_2_0)
.stringConf()
- .checkValue(StringUtils::isNotBlank,
ConfigConstants.NOT_BLANK_ERROR_MSG)
.create();
public static final ConfigEntry<String> CATALOG_URI =
diff --git
a/iceberg/iceberg-common/src/main/java/org/apache/gravitino/iceberg/common/ops/IcebergCatalogWrapper.java
b/iceberg/iceberg-common/src/main/java/org/apache/gravitino/iceberg/common/ops/IcebergCatalogWrapper.java
index 05c9ee2a1..0ed62b26f 100644
---
a/iceberg/iceberg-common/src/main/java/org/apache/gravitino/iceberg/common/ops/IcebergCatalogWrapper.java
+++
b/iceberg/iceberg-common/src/main/java/org/apache/gravitino/iceberg/common/ops/IcebergCatalogWrapper.java
@@ -29,6 +29,7 @@ import java.util.Set;
import java.util.function.Supplier;
import lombok.Getter;
import lombok.Setter;
+import org.apache.commons.lang3.StringUtils;
import org.apache.gravitino.catalog.lakehouse.iceberg.IcebergConstants;
import org.apache.gravitino.iceberg.common.IcebergCatalogBackend;
import org.apache.gravitino.iceberg.common.IcebergConfig;
@@ -82,9 +83,14 @@ public class IcebergCatalogWrapper implements AutoCloseable {
this.catalogBackend =
IcebergCatalogBackend.valueOf(
icebergConfig.get(IcebergConfig.CATALOG_BACKEND).toUpperCase(Locale.ROOT));
- if (!IcebergCatalogBackend.MEMORY.equals(catalogBackend)) {
+ if (!IcebergCatalogBackend.MEMORY.equals(catalogBackend)
+ && !IcebergCatalogBackend.REST.equals(catalogBackend)) {
// check whether IcebergConfig.CATALOG_WAREHOUSE exists
- icebergConfig.get(IcebergConfig.CATALOG_WAREHOUSE);
+ if
(StringUtils.isBlank(icebergConfig.get(IcebergConfig.CATALOG_WAREHOUSE))) {
+ throw new IllegalArgumentException("The 'warehouse' parameter must
have a value.");
+ }
+ }
+ if (!IcebergCatalogBackend.MEMORY.equals(catalogBackend)) {
this.catalogUri = icebergConfig.get(IcebergConfig.CATALOG_URI);
}
this.catalog = IcebergCatalogUtil.loadCatalogBackend(catalogBackend,
icebergConfig);
