[GitHub] [superset] villebro commented on a diff in pull request #23217: fix: add disallowed query params for engines specs

["villebro (via GitHub)" <gi...@apache.org>]

villebro commented on code in PR #23217:
URL: https://github.com/apache/superset/pull/23217#discussion_r1119664208


##########
superset/db_engine_specs/base.py:
##########
@@ -1724,6 +1726,18 @@ def get_public_information(cls) -> Dict[str, Any]:
             "disable_ssh_tunneling": cls.disable_ssh_tunneling,
         }
 
+    @classmethod
+    def validate_database_uri(cls, sqlalchemy_uri: URL) -> None:
+        """
+        Validates a database SQLAlchemy URI per engine spec.
+        Use this to implement a final validation for unwanted connection configuration
+
+        :param sqlalchemy_uri:
+        """
+        for query_param in sqlalchemy_uri.query.keys():
+            if query_param in cls.disallow_uri_query_params:
+                raise ValueError("Disallowed query parameter")

Review Comment:
   I agree with @Antonio-RiveroMartnez , it would be nice to list the query params that are disallowed. To do it all in one go one could use `set1.intersection(set2)` to get all overlapping disallowed params to avoid iterating over the list.



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