[GitHub] [superset] betodealmeida commented on a diff in pull request #21486: feat: add denylist for db engines

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

betodealmeida commented on code in PR #21486:
URL: https://github.com/apache/superset/pull/21486#discussion_r980601216


##########
tests/unit_tests/conftest.py:
##########
@@ -82,6 +82,11 @@ def app() -> Iterator[SupersetApp]:
     app.config["PREVENT_UNSAFE_DB_CONNECTIONS"] = False
     app.config["TESTING"] = True
 
+    # loop over extra configs passed in by tests
+    if request and hasattr(request, "param"):
+        for key, val in request.param.items():
+            app.config[key] = val
+

Review Comment:
   Oh, I like the idea!
   
   Should we give this a more descriptive name, just in case? Eg, `override_app_config` instead of `param`?
   
   Also, how is this used? Is `request` a Flask request object?



##########
superset/config.py:
##########
@@ -1102,6 +1103,13 @@ def EMAIL_HEADER_MUTATOR(  # pylint: disable=invalid-name,unused-argument
     return msg
 
 
+# For database connections, this dictionary will remove engines from the available
+# list/dropdown if you do not want these dbs to show as available.
+# The available list is generated by driver installed, and some engines have multiple
+# drivers.
+# e.g., DBS_AVAILABLE_DENYLIST: Dict[str, Tuple[str, ...]] = {"databricks": ("pyhive", "pyodbc")}
+DBS_AVAILABLE_DENYLIST: Dict[str, Tuple[str, ...]] = {}

Review Comment:
   Nit, I would make this a `Dict[str, Set[str]]`



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