villebro commented on a change in pull request #17593:
URL: https://github.com/apache/superset/pull/17593#discussion_r759906271



##########
File path: docs/src/pages/docs/Connecting to Databases/trino.mdx
##########
@@ -8,20 +8,77 @@ version: 1
 
 ## Trino
 
-Supported trino version 352 and higher
-
-The [sqlalchemy-trino](https://pypi.org/project/sqlalchemy-trino/) library is 
the recommended way to connect to Trino through SQLAlchemy.
-
-The expected connection string is formatted as follows:
+SuperSet supported Trino >=352 via SQLAlchemy by using 
[sqlalchemy-trino](https://pypi.org/project/sqlalchemy-trino/) library.
 
+### Connection String
+The connection string format is as follows:
 ```
 trino://{username}:{password}@{hostname}:{port}/{catalog}
 ```
-If you are running trino with docker on local machine please use the following 
connection URL
 
+If you are running trino with docker on local machine, please use the 
following connection URL

Review comment:
       nit:
   ```suggestion
   If you are running Trino with docker on local machine, please use the 
following connection URL
   ```

##########
File path: docs/src/pages/docs/Connecting to Databases/trino.mdx
##########
@@ -8,20 +8,77 @@ version: 1
 
 ## Trino
 
-Supported trino version 352 and higher
-
-The [sqlalchemy-trino](https://pypi.org/project/sqlalchemy-trino/) library is 
the recommended way to connect to Trino through SQLAlchemy.
-
-The expected connection string is formatted as follows:
+SuperSet supported Trino >=352 via SQLAlchemy by using 
[sqlalchemy-trino](https://pypi.org/project/sqlalchemy-trino/) library.
 
+### Connection String
+The connection string format is as follows:
 ```
 trino://{username}:{password}@{hostname}:{port}/{catalog}
 ```
-If you are running trino with docker on local machine please use the following 
connection URL
 
+If you are running trino with docker on local machine, please use the 
following connection URL
 ```
 trino://[email protected]:8080
 ```
 
-Reference:
-[Trino-Superset-Podcast](https://trino.io/episodes/12.html)
+### Authentications
+#### 1. Basic Authentication
+You can provide `username`/`password` in connection string or in `Secure 
Extra` field at `Advanced / Security`
+* In Connection String
+    ```
+    trino://{username}:{password}@{hostname}:{port}/{catalog}
+    ```
+
+* In `Secure Extra` field
+    ```json
+    {
+        "auth_method": "basic",
+        "auth_params": {
+            "username": "<username>",
+            "password": "<password>"
+        }
+    }
+    ```
+
+NOTE: if both are provided, `Secure Extra` always take higher priority.
+
+#### 2. Kerberos Authentication
+In `Secure Extra` field, config as following example:
+```json
+{
+    "auth_method": "kerberos",
+    "auth_params": {
+        "service_name": "SuperSet",
+        "config": "/path/to/krb5.config",
+        ...
+    }
+}
+```
+
+All fields in `auth_params` are pass directly to 
[`KerberosAuthentication`](https://github.com/trinodb/trino-python-client/blob/0.306.0/trino/auth.py#L40)
 class.

Review comment:
       nit:
   ```suggestion
   All fields in `auth_params` are passed directly to 
[`KerberosAuthentication`](https://github.com/trinodb/trino-python-client/blob/0.306.0/trino/auth.py#L40)
 class.
   ```

##########
File path: tests/integration_tests/db_engine_specs/trino_tests.py
##########
@@ -87,3 +89,71 @@ def test_get_extra_params_with_server_cert(self, 
create_ssl_cert_file_func: Mock
         self.assertEqual(connect_args.get("http_scheme"), "https")
         self.assertEqual(connect_args.get("verify"), "/path/to/tls.crt")
         create_ssl_cert_file_func.assert_called_once_with(database.server_cert)
+
+    @patch("trino.auth.BasicAuthentication")
+    def test_auth_basic(self, auth: Mock):
+        database = Mock()
+
+        auth_params = dict(username="username", password="password",)
+        database.encrypted_extra = json.dumps(
+            dict(auth_method="basic", auth_params=auth_params)
+        )
+
+        params: Dict[str, Any] = {}
+        TrinoEngineSpec.update_encrypted_extra_params(database, params)
+        connect_args = params.setdefault("connect_args", {})
+        self.assertEqual(connect_args.get("http_scheme"), "https")
+        auth.assert_called_once_with(**auth_params)
+
+    @patch("trino.auth.KerberosAuthentication")
+    def test_auth_kerberos(self, auth: Mock):
+        database = Mock()
+
+        auth_params = dict(
+            service_name="superset", mutual_authentication=False, 
delegate=True,
+        )
+        database.encrypted_extra = json.dumps(
+            dict(auth_method="kerberos", auth_params=auth_params)
+        )

Review comment:
       Same here




-- 
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