villebro commented on a change in pull request #13214:
URL: https://github.com/apache/superset/pull/13214#discussion_r580070291
##########
File path: superset/db_engine_specs/base.py
##########
@@ -909,19 +909,22 @@ def modify_url_for_impersonation(
url.username = username
@classmethod
- def get_configuration_for_impersonation( # pylint: disable=invalid-name
- cls, uri: str, impersonate_user: bool, username: Optional[str]
- ) -> Dict[str, str]:
+ def update_connect_args_for_impersonation(
+ cls,
+ connect_args: Dict[Any, Any],
Review comment:
nit: `connect_args` should be of type `Dict[str, Any]`
##########
File path: superset/db_engine_specs/base.py
##########
@@ -909,19 +909,22 @@ def modify_url_for_impersonation(
url.username = username
@classmethod
- def get_configuration_for_impersonation( # pylint: disable=invalid-name
- cls, uri: str, impersonate_user: bool, username: Optional[str]
- ) -> Dict[str, str]:
+ def update_connect_args_for_impersonation(
+ cls,
+ connect_args: Dict[Any, Any],
+ uri: str,
+ impersonate_user: bool,
Review comment:
I would remove this `bool` from the signature and rather check it in
`core.py` prior to calling this method (no point in calling the method if we're
not impersonating).
##########
File path: superset/models/core.py
##########
@@ -325,16 +325,13 @@ def get_sqla_engine(
params["poolclass"] = NullPool
connect_args = params.get("connect_args", {})
- configuration = connect_args.get("configuration", {})
- # If using Hive, this will set
hive.server2.proxy.user=$effective_username
- configuration.update(
- self.db_engine_spec.get_configuration_for_impersonation(
- str(sqlalchemy_url), self.impersonate_user, effective_username
- )
+ # If using presto, this will set principal_username=$effective_username
+ # If using Hive, this will set
hive.server2.proxy.user=$effective_username on connect_args['configuration']
Review comment:
I would move this comment to `hive.py`, as `core.py` should be as
generic as possible.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
