Add support for Snowflake's "auth_jwt"
#85
Conversation
leans more on [adbc client options](https://arrow.apache.org/adbc/current/driver/snowflake.html#client-options) and Snowflake Driver's underlying `snowflakedb/gosnowflake` library
petermueller
force-pushed
the
snowflake-more-auth
branch
from
88313c8 to
f1a19a0
Compare
|
I have tested this extensively for the JWT approach, and is something needed in general since Snowflake is deprecating password-only sign-ins. I tried to set it up such that other auth'n types from the adbc Snowflake Driver docs could be implemented, but don't have the setup for testing those. Please let me know if there's any other testing you'd like to see. |
jonatanklosko
left a comment
jonatanklosko
left a comment
There was a problem hiding this comment.
One comment and we can ship it!
| "adbc.snowflake.sql.db": "default", | ||
| "adbc.snowflake.sql.schema": "schema", | ||
| "adbc.snowflake.sql.warehouse": "", | ||
| "adbc.snowflake.sql.auth_type": "auth_snowflake", |
There was a problem hiding this comment.
We should have a test for the code generated with auth_jwt as well :)
|
|
||
| assert ConnectionCell.to_source(put_in(attrs["type"], "snowflake")) == ~s''' | ||
| :ok = Adbc.download_driver!(:snowflake) | ||
| uri = "admin:pass@account/default/schema" |
There was a problem hiding this comment.
Btw, the URI can include parameters, so perhaps we could do ?warehouse={warehouse} (docs)? This way we can keep it as a URI, which is more condensed.
There was a problem hiding this comment.
There seems to be a bug upstream where if you provide adbc a URI and options related to other auth types, it will fail and claim that you need to provide a password. Interestingly, if you provide a dummy password in the URI, it will still connect.
This actually was the cause of a lot of this investigation, as I was previously converting the cell to source and then modifying it.
It seems that in adbc, the URI parsing is passed directly to the gosnowflake library (this was referenced in a closed issue as well, I'm on mobile but can edit once I find it, edit: this comment).
I read through a bunch of the gosnowflake code and I'm not sure if it's in that library or a combination of that library and it's usage in adbc, but if we build the URI ourselves then a lot of the auth options have to use undocumented query parameter names and values that aren't on the adbc documentation or the gosnowflake documentation.
I would also prefer building the URI ourselves, but mixing the adbc options in makes it difficult to consistently support and it seems like they are parsing the URI to get those options and then reconstructing it anyway so it seemed to me that the better option was to just use the options directly and remove the URI building in this library
There was a problem hiding this comment.
Got it, sounds good then!
3 participants
This leans on adbc client options for the URI creation and options, which in turn relies on the Snowflake Driver's upstream
snowflakedb/gosnowflakelibrary