Skip to content

Conversation

@smaheshwar-pltr
Copy link

Description

  1. The IcebergRestCatalogBackendContainer relies on the container deriving the AWS region from an environment variable, but this can be set instead with an Iceberg property, client.region
  2. The environment variable setting readability may be improved with a method that translates normal catalog properties to ones formatted how the container expects.

Additional context and related issues

Release notes

(x) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text:

## Section
* Fix some things. ({issue}`issuenumber`)

@cla-bot
Copy link

cla-bot bot commented Nov 23, 2025

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

"s3.session-token", minioSessionToken,
"s3.endpoint", "http://minio:4566",
"s3.path-style-access", "true",
"client.region", MINIO_REGION)),
Copy link
Author

@smaheshwar-pltr smaheshwar-pltr Nov 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can do this, using an explicit Iceberg property, instead of setting AWS_REGION as an env var for the AWS SDK to derive region from

@cla-bot
Copy link

cla-bot bot commented Nov 23, 2025

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

return getMappedHostAndPortForExposedPort(8181).toString();
}

private static Map<String, String> toCatalogEnvVars(Map<String, String> properties)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant