Skip to content

Conversation

@skrydal
Copy link
Collaborator

@skrydal skrydal commented Nov 13, 2025

Currently pyiceberg library does not use role-arn when connecting to Glue catalog (limits itself only to assume that role for s3 access). This is a workaround until such functionality is introduced in pyiceberg.

@github-actions github-actions bot added the ingestion PR or Issue related to the ingestion of metadata label Nov 13, 2025
@codecov
Copy link

codecov bot commented Nov 13, 2025

Codecov Report

❌ Patch coverage is 94.73684% with 2 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...datahub/ingestion/source/iceberg/iceberg_common.py 94.44% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

@skrydal skrydal marked this pull request as ready for review November 16, 2025 22:52
@datahub-cyborg datahub-cyborg bot added the needs-review Label for PRs that need review from a maintainer. label Nov 16, 2025
catalog_config, GLUE_ROLE_ARN, AWS_ROLE_ARN
)
if role_to_assume:
logger.debug(
Copy link
Contributor

Choose a reason for hiding this comment

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

this should probably be info level


sts_client = session.client("sts")
identity = sts_client.get_caller_identity()
logger.debug(
Copy link
Contributor

Choose a reason for hiding this comment

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

also info
IMO :-)

"Current role and the role we wanted to assume are the same, continuing without further assumption steps"
)
else:
logger.debug(f"Assuming the role {role_to_assume}")
Copy link
Contributor

Choose a reason for hiding this comment

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

info

response = sts_client.assume_role(
RoleArn=role_to_assume,
RoleSessionName="session",
DurationSeconds=43200,
Copy link
Contributor

Choose a reason for hiding this comment

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

this could be a constant and we could mention that 12h is the max we can set

RoleSessionName could be more descriptive

@skrydal skrydal merged commit 2296ce7 into master Nov 17, 2025
62 checks passed
@skrydal skrydal deleted the ps_iceberg_assume_role branch November 17, 2025 11:26
dgluong-datahub pushed a commit that referenced this pull request Nov 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ingestion PR or Issue related to the ingestion of metadata needs-review Label for PRs that need review from a maintainer.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants