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

["Antonio-RiveroMartnez (via GitHub)" <gi...@apache.org>]

Antonio-RiveroMartnez commented on code in PR #23217:
URL: https://github.com/apache/superset/pull/23217#discussion_r1119199538


##########
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:
   Hey! two questions:
   
   1. Could we indicate what `query_param` is disallowed so user can have a better idea of what to change in future requests?
   2. Based on the question above, what if multiple `query_params` are disallowed in a given URI? Shouldn't we return a list containing all of them instead?



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