villebro commented on code in PR #21180:
URL: https://github.com/apache/superset/pull/21180#discussion_r959140949
##########
superset/databases/commands/test_connection.py:
##########
@@ -73,8 +74,12 @@ def run(self) -> None:
database.set_sqlalchemy_uri(uri)
database.db_engine_spec.mutate_db_for_connection_test(database)
-
- engine = database.get_sqla_engine()
+
+ engine = None
+ if "snowflake" in str(url) and "privatekey" in str(url):
Review Comment:
@xiayanzheng yes - please see how the Trino connector is currently doing it:
In short here's what I think should work:
1. The user should add the private key and password in `encryption_extra`
similar to Trino:
https://superset.apache.org/docs/databases/trino#authentications . You are free
to choose the structure of the JSON object, but I think the format chosen by
the Trino spec is good, as it makes it easy to add additional authentication
types as they become supported later. It's important you also add documentation
for the precise format in which users should add this information to the JSON
object in this file:
https://github.com/apache/superset/blob/master/docs/docs/databases/snowflake.mdx
2. You move the cryptography logic to
`superset.db_engine_specs.snowflake.SnowflakeEngineSpec.update_encrypted_extra_params`
like Trino does it here:
https://github.com/apache/superset/blob/034ee1c3c1fda20b238ba1cc9093a4156d9fa01e/superset/db_engine_specs/trino.py#L183-L221
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]