[GitHub] mistercrunch commented on a change in pull request #3652: Authentication: Enable user impersonation for Superset to HiveServer2 using hive.server2.proxy.user (a.fernandez)

[gi...@git.apache.org]

mistercrunch commented on a change in pull request #3652: Authentication: Enable user impersonation for Superset to HiveServer2 using hive.server2.proxy.user (a.fernandez)
URL: https://github.com/apache/incubator-superset/pull/3652#discussion_r144464156
 
 

 ##########
 File path: superset/db_engine_specs.py
 ##########
 @@ -830,6 +854,34 @@ def _partition_query(
             cls, table_name, limit=0, order_by=None, filters=None):
         return "SHOW PARTITIONS {table_name}".format(**locals())
 
+    @classmethod
+    def modify_url_for_impersonation(cls, url, impersonate_user, username):
+        """
+        Modify the SQL Alchemy URL object with the user to impersonate if applicable.
+        :param url: SQLAlchemy URL object
+        :param impersonate_user: Bool indicating if impersonation is enabled
+        :param username: Effective username
+        """
+        if impersonate_user is True and "auth" in url.query.keys() and username is not None:
+            url.query["hive_server2_proxy_user"] = username
+
+    @classmethod
+    def get_uri_for_impersonation(cls, uri, impersonate_user, username):
+        """
+        Return a new URI string that allows for user impersonation.
+        :param uri: URI string
+        :param impersonate_user:  Bool indicating if impersonation is enabled
+        :param username: Effective username
+        :return: New URI string
+        """
+        new_uri = uri
+        url = make_url(uri)
+        backend_name = url.get_backend_name()
+
+        # Must be Hive connection, enable impersonation, and set param auth=LDAP|KERBEROS
+        if backend_name == "hive" and "auth" in url.query.keys() and impersonate_user is True and username is not None:
 
 Review comment:
   Some very long lines here. PEP8 says 80, our pylint say 90. If you want to lint your PR only you can `git diff master... | flake8 --diff` thought that's flake8 not pylint. There's also `git-lint` which can lint your diff as well.
 
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services