-
Notifications
You must be signed in to change notification settings - Fork 140
fix[s3 source]: allow for global or model config level definition of the role_arn #440
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR allows for the configuration of the S3 role ARN at either the global (project) or model level.
- Added a new integration test class (TestS3RoleArnGlobal) that confirms the compiled SQL contains the correct extra_credentials invocation.
- Updated the clickhouse adapter implementation to retrieve the role ARN from either the provided parameter or the project configuration.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
File | Description |
---|---|
tests/integration/adapter/clickhouse/test_clickhouse_s3.py | Introduces new integration tests to verify role ARN injection in compiled SQL. |
dbt/adapters/clickhouse/impl.py | Updates the S3 source clause generation to include role ARN from configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enhances S3 source configurations by allowing the role_arn to be defined globally or at the model level. Key changes include:
- Adding integration tests to verify the role_arn and AWS access key configurations.
- Updating the ClickHouse adapter's S3 source clause function to support role_arn from configuration.
- Ensuring that the SQL compilation properly includes the new role_arn and AWS credential format.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
tests/integration/adapter/clickhouse/test_clickhouse_s3.py | Added tests to validate AWS access key and role_arn configurations |
dbt/adapters/clickhouse/impl.py | Updated s3source_clause to integrate role_arn from config |
Hi @sarahdasko |
happy to @BentsiLeviav |
@sarahdasko would need you to merge it one more time :) |
Summary
Added
role_arn
to be configurable at the project or model levelAdded the ability to configure
aws_access_key_id
andaws_secret_access_key
at the project or model levelChecklist