Skip to content
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

Feature/dbt snowflake/connection_name parameter support #848

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

sfc-gh-dflippo
Copy link

resolves #684

Problem

Snowflake now supports a connection_name parameter and a connections.toml file with connection parameters. dbt does not currently support this feature. One of the advantages of the connections.toml is that it can leverage new snowflake features as they are introduced without requiring any direct changes in dbt.

Additional information on this parameter and the connections.toml file are available in the Snowflake documentation.

Essentially, by adding the connection_name parameter, dbt can avoid having to promptly implement most new parameters in the future. This change is also far less invasive and much easier to test than attempting to support all the individual authentication parameters supported by the Snowflake Python Connector. Testers won't need to test every individual parameter, just that dbt is able to pass the connection_name parameter to the Snowflake driver and that the driver is able to pick up parameters from the connections.toml that are not specified in the profile.yml. Those parameters can be as simple as a Snowflake password or the parameters for key pair authentication.

WIthout this change, features such as the token_file_path parameter cannot be used with dbt. That feature is important to Snowflake customers who wish to deploy dbt in a Snowpark Container and is the original reason I opened the issue. More importantly though, Snowflake frequently adds new parameters. You can see the current list, which is far longer than dbt supports, in the Snowflake connection.py script, on lines 325-381.

Solution

The only changes necessary are in the dbt-snowflake connections.py script. In three places, we just add the connection_name parameter the same as any other parameter. Then, I also updated the __post_init__ method to not perform as many checks on the authentication parameters when the connection_name is specified. This allows us to skip entering passwords or key pair details in dbt when we will instead put these settings in the connections.toml.

Testing

I have provided a test_connection_name.py unit test in the PR. I have tested a number of different parameters with my connections.toml and they are all picked up. I believe it is sufficient to test that dbt can pick up a simple parameter like password. Beyond that, the Snowflake developers will be responsible to test all the possible values for the connections.toml.

These are the instructions to test the feature, adapted from unit test script:

  • Create a connections.toml file at ~/.snowflake/connections.toml or you can specify a different folder using env variable SNOWFLAKE_HOME. You don't have to reference this location in dbt because the Snowflake driver will pick up this environment variable.

  • The file should have an entry similar to the following with your credentials. Any type of authentication can be used.

[default]
user = "test_user"
warehouse = "test_warehouse"
database = "test_database"
schema = "test_schema"
role = "test_role"
password = "test_password"
authenticator = "snowflake"
  • On Linux and Mac OS you will need to set the following permissions on your connections.toml or you will receive an error.
chown $USER ~/.snowflake/connections.toml
chmod 0600 ~/.snowflake/connections.toml
  • Specify your connection_name parameter in your profiles.yml config
  • Take out settings like password, private_key_path, or private_key_passphrase from your profiles.yml config
  • Test executing dbt debug and it will use the the credentials you added to the connections.toml to connect to Snowflake. As expected, you will not see these credentials in the output of dbt debug but you will see the value of the connection_name parameter.

Checklist

  • I have read the contributing guide and understand what's expected of me
  • I have run this code in development and it appears to resolve the stated issue
  • This PR includes tests, or tests are not required/relevant for this PR
  • This PR has no interface changes (e.g. macros, cli, logs, json artifacts, config files, adapter interface, etc) or this PR has already received feedback and approval from Product or DX

@sfc-gh-dflippo sfc-gh-dflippo requested a review from a team as a code owner February 24, 2025 20:41
Copy link

cla-bot bot commented Feb 24, 2025

Thanks for your pull request, and welcome to our community! We require contributors to sign our Contributor License Agreement and we don't seem to have your signature on file. Check out this article for more information on why we have a CLA.

In order for us to review and merge your code, please submit the Individual Contributor License Agreement form attached above above. If you have questions about the CLA, or if you believe you've received this message in error, please reach out through a comment on this PR.

CLA has not been signed by users: @sfc-gh-dflippo

Copy link
Contributor

Thank you for your pull request! We could not find a changelog entry for this change in the dbt-snowflake package. For details on how to document a change, see the Contributing Guide.

@sfc-gh-dflippo
Copy link
Author

I will review the new changelog instructions and add that to my PR. As we discussed in a different pull request, I am a Principal Solutions Architect with Snowflake and per Amy Chen, since we have a direct partnership, we can bypass the CLA but still go through the external PR review process.

Copy link

cla-bot bot commented Feb 24, 2025

Thanks for your pull request, and welcome to our community! We require contributors to sign our Contributor License Agreement and we don't seem to have your signature on file. Check out this article for more information on why we have a CLA.

In order for us to review and merge your code, please submit the Individual Contributor License Agreement form attached above above. If you have questions about the CLA, or if you believe you've received this message in error, please reach out through a comment on this PR.

CLA has not been signed by users: @sfc-gh-dflippo

Copy link

cla-bot bot commented Feb 24, 2025

Thanks for your pull request, and welcome to our community! We require contributors to sign our Contributor License Agreement and we don't seem to have your signature on file. Check out this article for more information on why we have a CLA.

In order for us to review and merge your code, please submit the Individual Contributor License Agreement form attached above above. If you have questions about the CLA, or if you believe you've received this message in error, please reach out through a comment on this PR.

CLA has not been signed by users: @sfc-gh-dflippo

Copy link

cla-bot bot commented Feb 24, 2025

Thanks for your pull request, and welcome to our community! We require contributors to sign our Contributor License Agreement and we don't seem to have your signature on file. Check out this article for more information on why we have a CLA.

In order for us to review and merge your code, please submit the Individual Contributor License Agreement form attached above above. If you have questions about the CLA, or if you believe you've received this message in error, please reach out through a comment on this PR.

CLA has not been signed by users: @sfc-gh-dflippo

Copy link

cla-bot bot commented Feb 25, 2025

Thanks for your pull request, and welcome to our community! We require contributors to sign our Contributor License Agreement and we don't seem to have your signature on file. Check out this article for more information on why we have a CLA.

In order for us to review and merge your code, please submit the Individual Contributor License Agreement form attached above above. If you have questions about the CLA, or if you believe you've received this message in error, please reach out through a comment on this PR.

CLA has not been signed by users: @sfc-gh-dflippo

Copy link

cla-bot bot commented Feb 25, 2025

Thanks for your pull request, and welcome to our community! We require contributors to sign our Contributor License Agreement and we don't seem to have your signature on file. Check out this article for more information on why we have a CLA.

In order for us to review and merge your code, please submit the Individual Contributor License Agreement form attached above above. If you have questions about the CLA, or if you believe you've received this message in error, please reach out through a comment on this PR.

CLA has not been signed by users: @sfc-gh-dflippo

Copy link

cla-bot bot commented Feb 25, 2025

Thanks for your pull request, and welcome to our community! We require contributors to sign our Contributor License Agreement and we don't seem to have your signature on file. Check out this article for more information on why we have a CLA.

In order for us to review and merge your code, please submit the Individual Contributor License Agreement form attached above above. If you have questions about the CLA, or if you believe you've received this message in error, please reach out through a comment on this PR.

CLA has not been signed by users: @sfc-gh-dflippo

Copy link

cla-bot bot commented Feb 25, 2025

Thanks for your pull request, and welcome to our community! We require contributors to sign our Contributor License Agreement and we don't seem to have your signature on file. Check out this article for more information on why we have a CLA.

In order for us to review and merge your code, please submit the Individual Contributor License Agreement form attached above above. If you have questions about the CLA, or if you believe you've received this message in error, please reach out through a comment on this PR.

CLA has not been signed by users: @sfc-gh-dflippo

Copy link

cla-bot bot commented Feb 25, 2025

Thanks for your pull request, and welcome to our community! We require contributors to sign our Contributor License Agreement and we don't seem to have your signature on file. Check out this article for more information on why we have a CLA.

In order for us to review and merge your code, please submit the Individual Contributor License Agreement form attached above above. If you have questions about the CLA, or if you believe you've received this message in error, please reach out through a comment on this PR.

CLA has not been signed by users: @sfc-gh-dflippo

Copy link

cla-bot bot commented Feb 25, 2025

Thanks for your pull request, and welcome to our community! We require contributors to sign our Contributor License Agreement and we don't seem to have your signature on file. Check out this article for more information on why we have a CLA.

In order for us to review and merge your code, please submit the Individual Contributor License Agreement form attached above above. If you have questions about the CLA, or if you believe you've received this message in error, please reach out through a comment on this PR.

CLA has not been signed by users: @sfc-gh-dflippo

Copy link

cla-bot bot commented Feb 25, 2025

Thanks for your pull request, and welcome to our community! We require contributors to sign our Contributor License Agreement and we don't seem to have your signature on file. Check out this article for more information on why we have a CLA.

In order for us to review and merge your code, please submit the Individual Contributor License Agreement form attached above above. If you have questions about the CLA, or if you believe you've received this message in error, please reach out through a comment on this PR.

CLA has not been signed by users: @sfc-gh-dflippo

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.

[Feature] Add Support for the token_file_path parameter used for OAuth in Snowpark Container Services
1 participant