Snowflake: automatic clustering + SSO updates #262
Conversation
|
thanks for opening this PR @bessey! I didn't catch that we need to upgrade dbt to use I did see that the links you shared are for the ODBC driver - do you happen to know if there are corresponding docs around the snowflake-connector-python library? |
|
To be honest I am just repeating a comment I read on the internet 😅 snowflakedb/snowflake-connector-python#140 (comment) I have not tested wether this does / doesn't work on 2.2.8 vs earlier versions. My work machine is Linux, so I can't test easily now I know that its Windows / Mac only. |
|
Looks like 2.2.3 does the job from the release notes: link |
|
ah, ok, got it. I'll queue up an issue in dbt core to update our snowflake-connector-python version to >= 2.2.3. If you don't mind, I'd like to leave this PR open until we've had a chance to dig in and verify that this works as expected! We fortunately have a mac we can test this out with :) |
|
just opened up the relevant issue over here: dbt-labs/dbt-core#2613 |
|
@drewbanin — do we still want to merge this PR? Can you either:
|
There was a problem hiding this comment.
@bessey Thanks for this!
I left one small comment on the Snowflake clustering change, which looks great otherwise. There are a few more steps around the SSO caching docs since this is prerelease functionality.
(thanks for the nudge @clrcrl @drewbanin)
| @@ -120,7 +120,9 @@ create or replace table my_database.my_schema.my_table as ( | |||
|
|
|||
| ### Automatic clustering | |||
|
|
|||
| Automatic clustering is a preview feature in Snowflake (at the time of this writing) and as such, some accounts may have it turned on while others may not. You can use the `automatic_clustering` config to control whether or not automatic clustering is enabled for dbt models. When `automatic_clustering` is set to `true`, dbt will run an `alter table <table name> resume recluster` query after building the target table. This configuration is only required for Snowflake accounts which do not have automatic clustering enabled. For more information, consult the [Snowflake documentation on Manual Reclustering](https://docs.snowflake.net/manuals/user-guide/tables-clustering-manual.html#switching-from-manual-reclustering-to-automatic-clustering). | |||
| Automatic clustering is [enabled by default in Snowflake today](https://docs.snowflake.com/en/user-guide/tables-clustering-manual.html), no action is needed to make use of it. Though there is an `automatic_clustering` config, it has no effect except for accounts with (deprecated) manual clustering enabled. | |||
There was a problem hiding this comment.
Did you mean to link to the docs on automatic clustering here? I see you linked to the manual clustering docs below
| Automatic clustering is [enabled by default in Snowflake today](https://docs.snowflake.com/en/user-guide/tables-clustering-manual.html), no action is needed to make use of it. Though there is an `automatic_clustering` config, it has no effect except for accounts with (deprecated) manual clustering enabled. | |
| Automatic clustering is [enabled by default in Snowflake today](https://docs.snowflake.com/en/user-guide/tables-auto-reclustering.html), no action is needed to make use of it. Though there is an `automatic_clustering` config, it has no effect except for accounts with (deprecated) manual clustering enabled. |
There was a problem hiding this comment.
Yeah, I don't think I meant to do this, I've just taken another look at the old link and it seems more applicable, reverted that.
| @@ -66,7 +66,7 @@ my-snowflake-db: | |||
|
|
|||
| To use SSO authentication for Snowflake, omit a `password` and instead supply an `authenticator` config to your target. `authenticator` can be one of 'externalbrowser' or a valid Okta URL. | |||
|
|
|||
| **Note**: By default, every connection that dbt opens will require you to re-authenticate in a browser. Contact your Snowflake support rep and inquire about turning on the "id token cache" for your account as described [here](https://github.com/snowflakedb/snowflake-connector-python/issues/140#issuecomment-447028785). | |||
| **Note**: By default, every connection that dbt opens will require you to re-authenticate in a browser. The Snowflake connector package supports cacheing your session token, but it [currently only supports Windows and Mac OS](https://docs.snowflake.com/en/user-guide/odbc-parameters.html#using-browser-based-sso-with-connection-caching-macos-and-windows-only). See [the Snowflake docs](https://docs.snowflake.com/en/user-guide/odbc-parameters.html#using-browser-based-sso-with-connection-caching-macos-and-windows-only) for how to enable this feature in your account. | |||
There was a problem hiding this comment.
We've upgraded the version of snowflake-connector-python and enabled the token cache, but that change is coming in v0.18.0 (currently a release candidate). As such, can you:
- switch the base branch of this PR to the
nextbranch - add
<Changelog>New in v0.18.0</Changelog>above the section about caching - link this page in the "New and changed documentation" section of the 0-18-0 migration guide
There was a problem hiding this comment.
Sorry, I was on holiday over the past 2 weeks so it looks like I missed the boat, nevertheless I've updated the base to current as (I think?) you've changed the base branch?
PS, might be helpful to warn the others targeting master still, it took me a while to work out why I was getting horrific conflicts after I switched to next and then tried to switch back to the old base.
I have not actually used this feature yet, but just reading the docs they appear to be out of line with Snowflake's https://docs.snowflake.com/en/user-guide/tables-clustering-manual.html
REPO SYNC - Public to Private
Description & motivation
As requested by @drewbanin dbt-labs/dbt-core#1172 (comment).
The SSO docs reflect my and other experience. I have no included the part about overriding the snowflake-connector-python to >=2.2.8, I am hoping a dbt author can just update 1.18 to use this version instead?
Full disclosure, I've not used automatic clustering, but i skimmed the docs for other stuff that looked like it could be out of date, and that stuck out. Since May 2020 its on by default for everyone, and I notice the current implementation toggles between "the account default" and "on", so effectively there's no way to disable it anymore. The docs reflect that now.
To-do before merge