[GitHub] [superset] mistercrunch commented on a change in pull request #16346: feat: trino support server-cert

[GitBox <gi...@apache.org>]

mistercrunch commented on a change in pull request #16346:
URL: https://github.com/apache/superset/pull/16346#discussion_r691801852



##########
File path: superset/db_engine_specs/base.py
##########
@@ -1250,6 +1249,22 @@ def get_extra_params(database: "Database") -> Dict[str, Any]:
                 raise ex
         return extra
 
+    @staticmethod
+    def get_encrypted_extra_params(database: "Database") -> Dict[str, Any]:

Review comment:
       Why moving this logic from Database model to here? At first I thought you'd be overriding it for Trino but then not, so I'm not clear why moving the logic to `BaseEngineSpec`?

##########
File path: tests/integration_tests/db_engine_specs/trino_tests.py
##########
@@ -14,11 +14,35 @@
 # KIND, either express or implied.  See the License for the
 # specific language governing permissions and limitations
 # under the License.
+import json
+from unittest.mock import Mock
+
 from sqlalchemy.engine.url import URL
 
 from superset.db_engine_specs.trino import TrinoEngineSpec
 from tests.integration_tests.db_engine_specs.base_tests import TestDbEngineSpec
 
+TEST_CERT = """\
+-----BEGIN CERTIFICATE-----

Review comment:
       wondering if some security automation system on our build systems at Preset is going to trigger while seeing this as it looks like stuff that shouldn't get checked into a repo




-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org