Add read support for S3 Tables in Iceberg #24815
Conversation
ebyhr
force-pushed
the
ebi/iceberg-s3-tables
branch
from
74bf75c to
f74a78e
Compare
ebyhr
changed the title
ebyhr
force-pushed
the
ebi/iceberg-s3-tables
branch
from
f74a78e to
18c568e
Compare
ebyhr
changed the title
ebyhr
force-pushed
the
ebi/iceberg-s3-tables
branch
from
18c568e to
a661bf4
Compare
ebyhr
force-pushed
the
ebi/iceberg-s3-tables
branch
from
a661bf4 to
f543bc5
Compare
ebyhr
added
the
needs-docs
ebyhr
requested review from
wendigo,
lukasz-walkiewicz,
mayankvadariya and
pajaks
| # Iceberg Connector Developer Notes | ||
|
|
||
| Steps to create TPCH tables on S3 Tables: | ||
| 1. Set `AWS_REGION`, `AWS_ACCESS_KEY_ID` and `AWS_SECRET_ACCESS_KEY` environment variables. |
There was a problem hiding this comment.
do we want to document how to set up S3 table user, lake formation permissions etc.? at least some docs to follow in to set up the environment.
There was a problem hiding this comment.
Yes, but it's purely AWS setup information, not specific to our test. It would nice to add a link to the official documentation once AWS publishes it.
...ceberg/src/main/java/io/trino/plugin/iceberg/catalog/rest/IcebergRestCatalogSigv4Config.java
Outdated
Show resolved
Hide resolved
plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/rest/SigV4AwsProperties.java
Outdated
Show resolved
Hide resolved
...ino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/rest/IcebergRestCatalogModule.java
Outdated
Show resolved
Hide resolved
plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/rest/SigV4AwsProperties.java
Outdated
Show resolved
Hide resolved
| .put("rest.access-key-id", s3Config.getAwsAccessKey()) | ||
| .put("rest.secret-access-key", s3Config.getAwsSecretKey()) |
There was a problem hiding this comment.
S3FileSystemConfig allows providing IAM role without requiring IAM user keys. But seems S3 table doesn't support Iam role, right?(please confirm) hence confirming if keys are provided.
requireNonNull(s3Config.getAwsAccessKey(), ...)
requireNonNull(s3Config.getAwsSecretKey(), ...)
There was a problem hiding this comment.
For iam role possibly here we need to assume the role and get the access key, secret access key and session token from there and pass it along to Iceberg lib.
There was a problem hiding this comment.
I assume we verified that IAM role is not supported.
...rc/test/java/io/trino/plugin/iceberg/catalog/rest/TestIcebergS3TablesConnectorSmokeTest.java
Outdated
Show resolved
Hide resolved
...ino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/rest/IcebergRestCatalogModule.java
Outdated
Show resolved
Hide resolved
ebyhr
force-pushed
the
ebi/iceberg-s3-tables
branch
from
f543bc5 to
45ccb7b
Compare
...ino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/rest/IcebergRestCatalogConfig.java
Outdated
Show resolved
Hide resolved
plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/rest/AwsProperties.java
Show resolved
Hide resolved
plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/rest/SigV4AwsProperties.java
Outdated
Show resolved
Hide resolved
...ceberg/src/main/java/io/trino/plugin/iceberg/catalog/rest/IcebergRestCatalogSigV4Config.java
Show resolved
Hide resolved
ebyhr
force-pushed
the
ebi/iceberg-s3-tables
branch
from
45ccb7b to
f575137
Compare
| return sigV4Enabled; | ||
| } | ||
|
|
||
| @Config("iceberg.rest-catalog.sigv4-enabled") |
There was a problem hiding this comment.
I assume there would be a separate ticket for updating the docs
Description
Supporting only read operations because Iceberg REST catalog in Glue doesn't support
stage-createoption Trino Iceberg connector depends on.This PR adds support for SigV4. Also, make
view-endpoints-supportedconfigurable because S3 Tables doesn't support view endpoints.Fixes #24679
Release notes