This is an automated email from the ASF dual-hosted git repository. diqiu50 pushed a commit to branch glue-pr03 in repository https://gitbox.apache.org/repos/asf/gravitino.git
commit 3288332af1f5339136e53e9743e40ba6b9e85e9f Author: diqiu50 <[email protected]> AuthorDate: Mon Apr 13 11:32:13 2026 +0800 refactor(catalog-glue): Address review comments - Rename TABLE_TYPE_FILTER_ALL to DEFAULT_TABLE_TYPE_FILTER - Add default credential chain order comment in GlueClientProvider - Remove try-catch wrapping on URI.create for endpoint validation - Make aws-glue-catalog-id optional - Add casing note to TABLE_FORMAT_TYPE JavaDoc - Clarify deferred validation for default-table-format and table-type-filter - Add StringUtils.isNotBlank() for blank value checks --- .../catalog/glue/GlueCatalogPropertiesMetadata.java | 10 ++++------ .../gravitino/catalog/glue/GlueClientProvider.java | 18 ++++++++---------- .../apache/gravitino/catalog/glue/GlueConstants.java | 2 +- .../catalog/glue/GlueTablePropertiesMetadata.java | 2 +- .../glue/TestGlueCatalogPropertiesMetadata.java | 4 ++-- .../gravitino/catalog/glue/TestGlueClientProvider.java | 4 +--- 6 files changed, 17 insertions(+), 23 deletions(-) diff --git a/catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueCatalogPropertiesMetadata.java b/catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueCatalogPropertiesMetadata.java index b2d631763e..c57ef79bad 100644 --- a/catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueCatalogPropertiesMetadata.java +++ b/catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueCatalogPropertiesMetadata.java @@ -25,8 +25,8 @@ import static org.apache.gravitino.catalog.glue.GlueConstants.AWS_REGION; import static org.apache.gravitino.catalog.glue.GlueConstants.AWS_SECRET_ACCESS_KEY; import static org.apache.gravitino.catalog.glue.GlueConstants.DEFAULT_TABLE_FORMAT; import static org.apache.gravitino.catalog.glue.GlueConstants.DEFAULT_TABLE_FORMAT_VALUE; +import static org.apache.gravitino.catalog.glue.GlueConstants.DEFAULT_TABLE_TYPE_FILTER; import static org.apache.gravitino.catalog.glue.GlueConstants.TABLE_TYPE_FILTER; -import static org.apache.gravitino.catalog.glue.GlueConstants.TABLE_TYPE_FILTER_ALL; import static org.apache.gravitino.connector.PropertyEntry.stringOptionalPropertyEntry; import static org.apache.gravitino.connector.PropertyEntry.stringRequiredPropertyEntry; @@ -87,8 +87,7 @@ public class GlueCatalogPropertiesMetadata extends BaseCatalogPropertiesMetadata DEFAULT_TABLE_FORMAT, stringOptionalPropertyEntry( DEFAULT_TABLE_FORMAT, - "Default format for tables created via createTable(). Accepted: iceberg, hive." - + " Unrecognised values are rejected at createTable() time.", + "Default format for tables created via createTable(). Accepted: iceberg, hive.", false /* immutable */, DEFAULT_TABLE_FORMAT_VALUE, false /* hidden */)) @@ -97,10 +96,9 @@ public class GlueCatalogPropertiesMetadata extends BaseCatalogPropertiesMetadata stringOptionalPropertyEntry( TABLE_TYPE_FILTER, "Comma-separated table types exposed by listTables() and loadTable()." - + " Accepted: all, hive, iceberg, delta, parquet." - + " Unrecognised values are rejected at listTables() time.", + + " Accepted: all, hive, iceberg.", false /* immutable */, - TABLE_TYPE_FILTER_ALL, + DEFAULT_TABLE_TYPE_FILTER, false /* hidden */)) .build(); diff --git a/catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueClientProvider.java b/catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueClientProvider.java index 3cf3e36036..536a3955bc 100644 --- a/catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueClientProvider.java +++ b/catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueClientProvider.java @@ -65,6 +65,13 @@ public final class GlueClientProvider { // Static credentials take priority over the default credential chain. // Both keys must be provided together — a partial pair is always a misconfiguration. + // Default credential chain order (when both keys are omitted): + // 1. Java system properties (aws.accessKeyId / aws.secretAccessKey) + // 2. Environment variables (AWS_ACCESS_KEY_ID / AWS_SECRET_ACCESS_KEY) + // 3. Web identity token (EKS / Kubernetes) + // 4. ~/.aws/credentials profile file + // 5. ECS container task role + // 6. EC2 instance profile (IMDSv2) String accessKey = config.get(GlueConstants.AWS_ACCESS_KEY_ID); String secretKey = config.get(GlueConstants.AWS_SECRET_ACCESS_KEY); boolean hasAccessKey = StringUtils.isNotBlank(accessKey); @@ -87,16 +94,7 @@ public final class GlueClientProvider { // Optional custom endpoint override for VPC endpoints or LocalStack testing. String endpoint = config.get(GlueConstants.AWS_GLUE_ENDPOINT); if (StringUtils.isNotBlank(endpoint)) { - try { - builder.endpointOverride(URI.create(endpoint)); - } catch (IllegalArgumentException e) { - throw new IllegalArgumentException( - String.format( - "Property '%s' contains an invalid URI: '%s'. " - + "Expected a valid URL, e.g. 'http://localhost:4566'. Cause: %s", - GlueConstants.AWS_GLUE_ENDPOINT, endpoint, e.getMessage()), - e); - } + builder.endpointOverride(URI.create(endpoint)); } return builder.build(); diff --git a/catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueConstants.java b/catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueConstants.java index f3ef784222..6ff98ebc28 100644 --- a/catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueConstants.java +++ b/catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueConstants.java @@ -63,7 +63,7 @@ public final class GlueConstants { public static final String TABLE_TYPE_FILTER = "table-type-filter"; /** Default value for {@link #TABLE_TYPE_FILTER}: expose all table types. */ - public static final String TABLE_TYPE_FILTER_ALL = "all"; + public static final String DEFAULT_TABLE_TYPE_FILTER = "all"; // ------------------------------------------------------------------------- // Glue Table.parameters() keys (passthrough properties) diff --git a/catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueTablePropertiesMetadata.java b/catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueTablePropertiesMetadata.java index 60399e444d..d6ddaa0202 100644 --- a/catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueTablePropertiesMetadata.java +++ b/catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueTablePropertiesMetadata.java @@ -47,7 +47,7 @@ public class GlueTablePropertiesMetadata extends BasePropertiesMetadata { stringOptionalPropertyEntry( TABLE_FORMAT_TYPE, "Glue table format type stored in Table.parameters(). Common values:" - + " ICEBERG, HIVE, DELTA, PARQUET.", + + " iceberg, hive.", false /* immutable */, null /* defaultValue */, false /* hidden */)) diff --git a/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGlueCatalogPropertiesMetadata.java b/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGlueCatalogPropertiesMetadata.java index 0cd1754f33..8d85653326 100644 --- a/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGlueCatalogPropertiesMetadata.java +++ b/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGlueCatalogPropertiesMetadata.java @@ -25,8 +25,8 @@ import static org.apache.gravitino.catalog.glue.GlueConstants.AWS_REGION; import static org.apache.gravitino.catalog.glue.GlueConstants.AWS_SECRET_ACCESS_KEY; import static org.apache.gravitino.catalog.glue.GlueConstants.DEFAULT_TABLE_FORMAT; import static org.apache.gravitino.catalog.glue.GlueConstants.DEFAULT_TABLE_FORMAT_VALUE; +import static org.apache.gravitino.catalog.glue.GlueConstants.DEFAULT_TABLE_TYPE_FILTER; import static org.apache.gravitino.catalog.glue.GlueConstants.TABLE_TYPE_FILTER; -import static org.apache.gravitino.catalog.glue.GlueConstants.TABLE_TYPE_FILTER_ALL; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -92,7 +92,7 @@ class TestGlueCatalogPropertiesMetadata { @Test void testTableTypeFilterDefaultValue() { assertEquals( - TABLE_TYPE_FILTER_ALL, + DEFAULT_TABLE_TYPE_FILTER, metadata.getDefaultValue(TABLE_TYPE_FILTER), "Default table type filter should be 'all'"); } diff --git a/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGlueClientProvider.java b/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGlueClientProvider.java index 5bccb0e138..f69a1c0739 100644 --- a/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGlueClientProvider.java +++ b/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGlueClientProvider.java @@ -116,8 +116,6 @@ class TestGlueClientProvider { config.put(AWS_REGION, "us-east-1"); config.put(AWS_GLUE_ENDPOINT, "not a valid uri ://"); - IllegalArgumentException ex = - assertThrows(IllegalArgumentException.class, () -> GlueClientProvider.buildClient(config)); - assertTrue(ex.getMessage().contains(AWS_GLUE_ENDPOINT)); + assertThrows(IllegalArgumentException.class, () -> GlueClientProvider.buildClient(config)); } }
