Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Conversation

@jrouly
Copy link
Contributor

@jrouly jrouly commented Mar 1, 2024

The upstream Iceberg documentation details glue.id as a parameter which can be used to set the AWS Glue Catalog ID (AWS Account ID).

This is critical for scenarios where the Glue catalog exists in a separate environment from the workload running pyiceberg (e.g. an account separation between workloads and data lake).

This PR reads glue.id and, if set, injects CatalogId= as a parameter on all boto3 Glue service calls.

pyiceberg/catalog/glue.py Show resolved Hide resolved
pyiceberg/catalog/glue.py Show resolved Hide resolved
@Fokko
Copy link
Contributor

Fokko commented Mar 2, 2024

@HonahX since you're more familiar with Glue, would you be able to do this review? :)

Copy link
Contributor

@HonahX HonahX left a comment

Choose a reason for hiding this comment

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

Overall LGTM! Thanks for working on this! @jrouly I like the way of using Event System to support the catalog id. Just have some small comments.

pyiceberg/catalog/glue.py Show resolved Hide resolved
pyiceberg/catalog/glue.py Outdated Show resolved Hide resolved
pyiceberg/catalog/glue.py Outdated Show resolved Hide resolved
pyiceberg/catalog/glue.py Show resolved Hide resolved
@jrouly
Copy link
Contributor Author

jrouly commented Mar 4, 2024

@HonahX appreciate the review! Pushed a new commit with your suggestions.

Copy link
Contributor

@HonahX HonahX left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks @jrouly

@HonahX HonahX merged commit be81529 into apache:main Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.