[Feature][Connector-Iceberg]Support iceberg rest catalog integration and support AWS S3 tables #9654
Conversation
davidzollo
left a comment
davidzollo
left a comment
There was a problem hiding this comment.
Good job.
Please add related E2E test case. thx
| .atZone(ZoneId.systemDefault()) | ||
| .withZoneSameInstant(ZoneOffset.UTC) | ||
| .toOffsetDateTime(); | ||
| return ((LocalDateTime) value).atOffset(ZoneOffset.UTC); |
There was a problem hiding this comment.
This is not right,
// In Beijing (UTC+8), developer writes:
LocalDateTime meeting = LocalDateTime.of(2024, 1, 1, 14, 0, 0); // 2 PM local
// Original code: Correctly converts to 6 AM UTC
// Modified code: Treats as 2 PM UTC (8 hours off!)
| // REST Catalog Configuration | ||
| public static final Option<String> REST_URI = | ||
| Options.key("rest.uri") | ||
| .stringType() | ||
| .noDefaultValue() | ||
| .withDescription("The URI for the REST catalog endpoint"); | ||
|
|
||
| public static final Option<String> REST_WAREHOUSE = | ||
| Options.key("rest.warehouse") | ||
| .stringType() | ||
| .noDefaultValue() | ||
| .withDescription("The warehouse location for the REST catalog"); | ||
|
|
||
| public static final Option<String> REST_AUTH_TYPE = | ||
| Options.key("rest.auth.type") | ||
| .stringType() | ||
| .defaultValue("none") | ||
| .withDescription( | ||
| "Authentication type for REST catalog. Supported values: none, token, aws"); | ||
|
|
||
| public static final Option<String> REST_AUTH_TOKEN = | ||
| Options.key("rest.auth.token") | ||
| .stringType() | ||
| .noDefaultValue() | ||
| .withDescription( | ||
| "Authentication token for REST catalog when auth.type is 'token'"); |
There was a problem hiding this comment.
why not directly put all config into iceberg.catalog.config?
| private void setupAwsAuthentication(Map<String, String> catalogProps) { | ||
| log.info("Setting up AWS authentication for REST catalog"); | ||
|
|
||
| // Enable SigV4 signing for AWS REST catalog | ||
| catalogProps.put("rest.sigv4-enabled", "true"); | ||
| log.info("Enabled SigV4 signing"); | ||
|
|
||
| // AWS credentials will be resolved through environment variables: | ||
| // AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY, AWS_SESSION_TOKEN (optional) | ||
| // AWS_REGION or AWS_DEFAULT_REGION | ||
| log.info("AWS credentials will be resolved through environment variables"); | ||
|
|
||
| // Determine signing service based on REST URI | ||
| String restUri = config.getRestUri(); | ||
| if (restUri != null) { | ||
| if (restUri.contains("s3tables")) { | ||
| catalogProps.put("rest.signing-name", "s3tables"); | ||
| log.info("Signing service set to: s3tables for URI: {}", restUri); | ||
| } else { | ||
| catalogProps.put("rest.signing-name", "glue"); | ||
| log.info("Signing service set to: glue for URI: {}", restUri); | ||
| } | ||
| } else { | ||
| catalogProps.put("rest.signing-name", "glue"); | ||
| log.warn("REST URI is null, defaulting signing service to: glue"); | ||
| } |
There was a problem hiding this comment.
Are our existing parameters already capable of reading AWS's rest catalog? Is this PR just a parameter conversion? I prefer to just add examples to the documentation rather than add a new config key.
There was a problem hiding this comment.
No, currently ST's Iceberg catalog only supports Hive and Hadoop, and does not support Rest. S3 tables is only a special Rest,and need to specify parameters for compatibility.
There was a problem hiding this comment.
IcebergCatalogType.java only worked for test case. So I think we support rest catalog. We have also accessed the rest catalog normally through the current version code in our environment.
There was a problem hiding this comment.
This is a demo:
iceberg.catalog.config = {
type=rest
uri="http://192.168.124.199:8080/catalog/"
warehouse=demo2
"s3.endpoint"="http://192.168.124.199:9000"
"s3.access-key-id"="******"
"s3.secret-access-key"="******"
io-impl="org.apache.iceberg.aws.s3.S3FileIO"
}
There was a problem hiding this comment.
3 participants
Purpose of this pull request
Does this PR introduce any user-facing change?
How was this patch tested?
Check list
New License Guide