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. You assume the user should add the private key and password in 
`encryption_extra` similar to Trino: 
https://superset.apache.org/docs/databases/trino#authentications . 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]

Reply via email to