You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2021/02/18 17:33:44 UTC

[GitHub] [superset] rijojoseph07 opened a new pull request #13214: fix(wip): Changes to support presto impersionation with ldap

rijojoseph07 opened a new pull request #13214:
URL: https://github.com/apache/superset/pull/13214


   ### SUMMARY
   Fix for issue https://github.com/apache/superset/issues/11359, https://github.com/apache/superset/issues/9406
   
   When using LDAP authentication with presto/trino, the current behavior is just to modify the URL to replace the username which will result in an unauthorized exception. This PR will fix this by updating the connection argument with the effective user. 
   
   
   ### TEST PLAN
   Step 1: Setup database connection to presto without credentials in URL.
   Step 2: Provide admin(who can impersonate as any other user in presto) credentials in connection properties via extras  
   ```
   "engine_params": {
              "connect_args":{
                 "protocol": "https",
                 "username":"<admin>",
                 "password":"<admin_password>"
              }
   }
   ```
   
   Step 3: Enable impersonation for the database connection. 
   Step 4: Log in with a different user and run the query via SQL lab and you will see the **principal user** as admin and **user** as the logged-in user in presto UI.
   
   ### ADDITIONAL INFORMATION
   <!--- Check any relevant boxes with "x" -->
   <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
   - [X] Has associated issue:
   - [ ] Changes UI
   - [ ] Requires DB Migration.
   - [ ] Confirm DB Migration upgrade and downgrade tested.
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] villebro merged pull request #13214: feat(presto): add support for user impersonation

Posted by GitBox <gi...@apache.org>.
villebro merged pull request #13214:
URL: https://github.com/apache/superset/pull/13214


   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] rijojoseph07 commented on pull request #13214: fix: Changes to support presto impersionation with ldap

Posted by GitBox <gi...@apache.org>.
rijojoseph07 commented on pull request #13214:
URL: https://github.com/apache/superset/pull/13214#issuecomment-782365870


   > A way forward if it's not possible to get the connect args out of SQLAlchemy would be to mock the `create_engine` call on line 364 in `superset/models/core.py` and assert on the arguments passed in. These systems for connecting to many different database types are quite challenging the community to test, so comprehensive unit test coverage is important for quality.
   
   @willbarrett Thanks for your suggestion. I have added a unit test by mocking `create_engine`. 


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] willbarrett commented on pull request #13214: fix: Changes to support presto impersionation with ldap

Posted by GitBox <gi...@apache.org>.
willbarrett commented on pull request #13214:
URL: https://github.com/apache/superset/pull/13214#issuecomment-782217532


   A way forward if it's not possible to get the connect args out of SQLAlchemy would be to mock the `create_engine` call on line 364 in `superset/models/core.py` and assert on the arguments passed in. These systems for connecting to many different database types are quite challenging the community to test, so comprehensive unit test coverage is important for quality.


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] DRavikanth commented on pull request #13214: feat(presto): add support for user impersonation

Posted by GitBox <gi...@apache.org>.
DRavikanth commented on pull request #13214:
URL: https://github.com/apache/superset/pull/13214#issuecomment-832280080


   @rijojoseph07, Yes, the user(**admin** is the user name in this case) creating this connection in Superset does exist in Presto and is enabled with impersonation. Please note that, I am able to login to Presto and execute the queries using this admin user with no issues. Also, please note that, if I uncheck Impersonation option in connection object, everything works as expected i.e the dashboards built on top of this connection will load the dashboards/charts from Presto.


-- 
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] villebro commented on a change in pull request #13214: fix: Changes to support presto impersionation with ldap

Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] rijojoseph07 commented on pull request #13214: feat(presto): add support for user impersonation

Posted by GitBox <gi...@apache.org>.
rijojoseph07 commented on pull request #13214:
URL: https://github.com/apache/superset/pull/13214#issuecomment-832456060


   @DRavikanth Can we connect over slack to discuss this in detail? Please join superset slack channel. 


-- 
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] willbarrett commented on pull request #13214: fix: Changes to support presto impersionation with ldap

Posted by GitBox <gi...@apache.org>.
willbarrett commented on pull request #13214:
URL: https://github.com/apache/superset/pull/13214#issuecomment-781666137


   @rijojoseph07 would it be add to add a test duplicating the bug to both verify the fix and prevent future regressions?


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] DRavikanth commented on pull request #13214: feat(presto): add support for user impersonation

Posted by GitBox <gi...@apache.org>.
DRavikanth commented on pull request #13214:
URL: https://github.com/apache/superset/pull/13214#issuecomment-832120089


   @rijojoseph07 I have exactly what you are testing against. I don't have Ranger. However, I don't think that's an issue. The only difference I see is the Presto version. I am using 341 and from the documentation I see impersonation is supported in that version. 
   
   I don't see any request being coming into the presto coordinator logs as well when the exception was initiated. I think this is failing in the UI itself and debug logs of Superset doesn't show anything related to this.
   
   I am looking into the UI Console and see the following:
   In Headers section:
   **Status:** 422 Unprocessable Entity
   
   In **Response:**
   message	"Connection failed, please check your connection settings"
   
   Request seems to be good though. Any help in debugging this further?


-- 
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] villebro commented on a change in pull request #13214: fix: Changes to support presto impersionation with ldap

Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #13214:
URL: https://github.com/apache/superset/pull/13214#discussion_r580053335



##########
File path: superset/db_engine_specs/base.py
##########
@@ -909,24 +909,9 @@ def modify_url_for_impersonation(
             url.username = username
 
     @classmethod
-    def get_configuration_for_impersonation(  # pylint: disable=invalid-name
+    def update_connect_args_for_impersonation(
         cls, uri: str, impersonate_user: bool, username: Optional[str]
-    ) -> Dict[str, str]:
-        """
-        Return a configuration dictionary that can be merged with other configs
-        that can set the correct properties for impersonating users
-
-        :param uri: URI
-        :param impersonate_user: Flag indicating if impersonation is enabled
-        :param username: Effective username
-        :return: Configs required for impersonation
-        """
-        return {}
-
-    @classmethod
-    def connect_arg_for_impersonation(
-        cls, uri: str, impersonate_user: bool, username: Optional[str]
-    ) -> Dict[str, str]:
+    ) -> Dict[str, Any]:

Review comment:
       I think we should add `connect_args` to the signature and mutate it here, as I'm not sure we can always call `connect_args.update()` in `core.py` (the required changes might not merge nicely into the original dict). Also, if we change it like this, the method doesn't need to return the object, as it's just mutating it within the method.




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] DRavikanth commented on pull request #13214: feat(presto): add support for user impersonation

Posted by GitBox <gi...@apache.org>.
DRavikanth commented on pull request #13214:
URL: https://github.com/apache/superset/pull/13214#issuecomment-831457448


   @rijojoseph07, I have upgraded to superset v1.1.0 & updated my connection settings & have corresponding impersonation settings in Presto as well. However, when clicking _**Impersonate logged in User button**_ on the database settings it throws an exception on the UI and nothing is visible in the backend logs. Details below:
   
   1. Upgraded to v1.1.0
   2. Presto 341 with impersonation rules as follows:
   `"impersonation": [
       {
         "originalUser": "admin",
         "newUser": ".*"
       }
     ],
   `
   Above means any user can be impersonated by admin user
   3. Removed username/password from DB Connection string
   4. Updated Extra as follows:
   `
   "connect_args": {
         "protocol": "https",
         "username":"admin",
         "password":"adminPwd",
         "session_props": {
             "omit_datetime_type_precision": true
         }
   `
   5. Check **_Impersonate Logged In User (Presto & Hive)_**
   6. Click on Save. UI throws following exception:
   
   > An error occurred while fetching databases: "Connection failed, please check your connection settings"
   


-- 
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] rijojoseph07 edited a comment on pull request #13214: feat(presto): add support for user impersonation

Posted by GitBox <gi...@apache.org>.
rijojoseph07 edited a comment on pull request #13214:
URL: https://github.com/apache/superset/pull/13214#issuecomment-831696748


   Hi @DRavikanth, were you able to connect using admin credentials without impersonation? Also it will be helpful if you can run superset in debug mode and share the logs. If possible share the complete connection extras after masking sensitive data.


-- 
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] rijojoseph07 commented on pull request #13214: feat(presto): add support for user impersonation

Posted by GitBox <gi...@apache.org>.
rijojoseph07 commented on pull request #13214:
URL: https://github.com/apache/superset/pull/13214#issuecomment-832150969


   @DRavikanth Can you please confirm if the user which is creating this dataset with impersonation exist is a valid user for presto ? 
   What I am trying to say is that : lets say you are logged in as `supersetadmin` and you add this dataset and test the connection, it will add `supersetadmin` as principal_username and if  `supersetadmin` is not a valid user to presto, this might fail. 


-- 
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] DRavikanth commented on pull request #13214: feat(presto): add support for user impersonation

Posted by GitBox <gi...@apache.org>.
DRavikanth commented on pull request #13214:
URL: https://github.com/apache/superset/pull/13214#issuecomment-831701915


   Yes the admin credentials without impersonation works with no issues.
   
   I'm running Superset in debug mode and I don't see any logs associated with
   this.
   
   Thanks,
   Ravi
   
   On Mon, May 3, 2021 at 22:46 rijojoseph07 ***@***.***> wrote:
   
   > Hi @DRavikanth <https://github.com/DRavikanth>, were you able to connect
   > using admin credentials without impersonation? Also it will be helpful if
   > you can run superset in debug mode and share the logs.
   >
   > —
   > You are receiving this because you were mentioned.
   > Reply to this email directly, view it on GitHub
   > <https://github.com/apache/superset/pull/13214#issuecomment-831696748>,
   > or unsubscribe
   > <https://github.com/notifications/unsubscribe-auth/ACRGFYILCSOHKHUGPSLITI3TL6C2PANCNFSM4X2W3ZIA>
   > .
   >
   


-- 
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] rijojoseph07 commented on pull request #13214: fix: Changes to support presto impersionation with ldap

Posted by GitBox <gi...@apache.org>.
rijojoseph07 commented on pull request #13214:
URL: https://github.com/apache/superset/pull/13214#issuecomment-783132514


   @willbarrett @villebro Can you guys please review this PR.
   
   Thanks


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] rijojoseph07 edited a comment on pull request #13214: feat(presto): add support for user impersonation

Posted by GitBox <gi...@apache.org>.
rijojoseph07 edited a comment on pull request #13214:
URL: https://github.com/apache/superset/pull/13214#issuecomment-831718900


   > Yes the admin credentials without impersonation works with no issues. I'm running Superset in debug mode and I don't see any logs associated with this. Thanks, Ravi
   > […](#)
   > On Mon, May 3, 2021 at 22:46 rijojoseph07 ***@***.***> wrote: Hi @DRavikanth <https://github.com/DRavikanth>, were you able to connect using admin credentials without impersonation? Also it will be helpful if you can run superset in debug mode and share the logs. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <[#13214 (comment)](https://github.com/apache/superset/pull/13214#issuecomment-831696748)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACRGFYILCSOHKHUGPSLITI3TL6C2PANCNFSM4X2W3ZIA> .
   
   @DRavikanth I just test this feature with superset 1.1.0 and presto 347 and it is working fine. Can you please check if impersonation is happening working as expected from the presto side via cli (this support was recently added to presto so you may have to upgrade presto)? 
   
   My connection URL looks like this : presto://presto-dns:443/hive/dev
   
   Extra : 
   
   {
           "metadata_params": {},
           "cost_estimate_enabled": true,
           "engine_params": {
              "connect_args":{
                 "protocol": "https",
                 "username":"admin",
                 "password":"<password>"
              }
            },
           "metadata_cache_timeout": {},
           "schemas_allowed_for_csv_upload": ["dev"]
       }
   
   I use ranger with presto, and have enabled impersonation from ranger.  You can also check presto server logs to see if your request is reaching presto. 
   


-- 
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] codecov-io commented on pull request #13214: fix: Changes to support presto impersionation with ldap

Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #13214:
URL: https://github.com/apache/superset/pull/13214#issuecomment-782387935


   # [Codecov](https://codecov.io/gh/apache/superset/pull/13214?src=pr&el=h1) Report
   > Merging [#13214](https://codecov.io/gh/apache/superset/pull/13214?src=pr&el=desc) (1b3f4fa) into [master](https://codecov.io/gh/apache/superset/commit/2ce79823dfad61bce6196fcacd56a844f44818c0?el=desc) (2ce7982) will **increase** coverage by `13.69%`.
   > The diff coverage is `52.36%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/13214/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/superset/pull/13214?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff             @@
   ##           master   #13214       +/-   ##
   ===========================================
   + Coverage   53.06%   66.75%   +13.69%     
   ===========================================
     Files         489      493        +4     
     Lines       17314    29069    +11755     
     Branches     4482        0     -4482     
   ===========================================
   + Hits         9187    19404    +10217     
   - Misses       8127     9665     +1538     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | cypress | `?` | |
   | python | `66.75% <52.36%> (?)` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/13214?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [superset/constants.py](https://codecov.io/gh/apache/superset/pull/13214/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29uc3RhbnRzLnB5) | `100.00% <ø> (ø)` | |
   | [superset/examples/birth\_names.py](https://codecov.io/gh/apache/superset/pull/13214/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvYmlydGhfbmFtZXMucHk=) | `73.19% <ø> (ø)` | |
   | [...43f2fdb\_add\_granularity\_to\_charts\_where\_missing.py](https://codecov.io/gh/apache/superset/pull/13214/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbWlncmF0aW9ucy92ZXJzaW9ucy8wNzBjMDQzZjJmZGJfYWRkX2dyYW51bGFyaXR5X3RvX2NoYXJ0c193aGVyZV9taXNzaW5nLnB5) | `0.00% <0.00%> (ø)` | |
   | [...s/260bf0649a77\_migrate\_x\_dateunit\_in\_time\_range.py](https://codecov.io/gh/apache/superset/pull/13214/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbWlncmF0aW9ucy92ZXJzaW9ucy8yNjBiZjA2NDlhNzdfbWlncmF0ZV94X2RhdGV1bml0X2luX3RpbWVfcmFuZ2UucHk=) | `0.00% <0.00%> (ø)` | |
   | [...ons/versions/41ce8799acc3\_rename\_pie\_label\_type.py](https://codecov.io/gh/apache/superset/pull/13214/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbWlncmF0aW9ucy92ZXJzaW9ucy80MWNlODc5OWFjYzNfcmVuYW1lX3BpZV9sYWJlbF90eXBlLnB5) | `0.00% <0.00%> (ø)` | |
   | [superset/connectors/sqla/views.py](https://codecov.io/gh/apache/superset/pull/13214/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL3ZpZXdzLnB5) | `62.74% <4.34%> (ø)` | |
   | [superset/views/datasource.py](https://codecov.io/gh/apache/superset/pull/13214/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YXNvdXJjZS5weQ==) | `88.70% <16.66%> (ø)` | |
   | [superset/viz.py](https://codecov.io/gh/apache/superset/pull/13214/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdml6LnB5) | `58.61% <27.27%> (ø)` | |
   | [superset/views/core.py](https://codecov.io/gh/apache/superset/pull/13214/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `71.80% <66.66%> (ø)` | |
   | [superset/charts/commands/exceptions.py](https://codecov.io/gh/apache/superset/pull/13214/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY2hhcnRzL2NvbW1hbmRzL2V4Y2VwdGlvbnMucHk=) | `92.85% <77.77%> (ø)` | |
   | ... and [937 more](https://codecov.io/gh/apache/superset/pull/13214/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/13214?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/13214?src=pr&el=footer). Last update [94893ff...1b3f4fa](https://codecov.io/gh/apache/superset/pull/13214?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] DRavikanth edited a comment on pull request #13214: feat(presto): add support for user impersonation

Posted by GitBox <gi...@apache.org>.
DRavikanth edited a comment on pull request #13214:
URL: https://github.com/apache/superset/pull/13214#issuecomment-831457448


   @rijojoseph07, I have upgraded to superset v1.1.0 & updated my connection settings & have corresponding impersonation settings in Presto as well. However, when clicking _**Impersonate logged in User button**_ on the database settings it throws an exception on the UI and nothing is visible in the backend logs. Details below:
   
   1. Upgraded to v1.1.0
   2. Presto 341 with impersonation rules as follows:
   `"impersonation": [
       {
         "originalUser": "admin",
         "newUser": ".*"
       }
     ],
   `
   Above means any user can be impersonated by admin user
   3. Removed username/password from DB Connection string
   4. Updated Extra as follows:
   `
   "connect_args": {
         "protocol": "https",
         "username":"admin",
         "password":"adminPwd",
         "session_props": {
             "omit_datetime_type_precision": true
         }
   `
   5. Check **_Impersonate Logged In User (Presto & Hive)_**
   6. Click on Save. UI throws following exception:
   
   > An error occurred while fetching databases: "Connection failed, please check your connection settings"
   
   This is very useful feature which I have been waiting for a long time. Any help in debugging this further will be of great help. Also, please let me know if you need any further information to understand the problem.


-- 
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] rijojoseph07 commented on pull request #13214: fix: Changes to support presto impersionation with ldap

Posted by GitBox <gi...@apache.org>.
rijojoseph07 commented on pull request #13214:
URL: https://github.com/apache/superset/pull/13214#issuecomment-781932852


   > @rijojoseph07 would it be add to add a test duplicating the bug to both verify the fix and prevent future regressions?
   
   @willbarrett I am trying to write a unit test for this but struggling to get connect_params from the SQLAlchemy engine object to check if the new `principal_username` is added. 
   
   I have tested this code in our environment and is running in our prod system. 


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] codecov-io edited a comment on pull request #13214: feat(presto): add support for user impersonation

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #13214:
URL: https://github.com/apache/superset/pull/13214#issuecomment-782387935


   # [Codecov](https://codecov.io/gh/apache/superset/pull/13214?src=pr&el=h1) Report
   > Merging [#13214](https://codecov.io/gh/apache/superset/pull/13214?src=pr&el=desc) (c082af8) into [master](https://codecov.io/gh/apache/superset/commit/2ce79823dfad61bce6196fcacd56a844f44818c0?el=desc) (2ce7982) will **increase** coverage by `13.61%`.
   > The diff coverage is `53.82%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/13214/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/superset/pull/13214?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff             @@
   ##           master   #13214       +/-   ##
   ===========================================
   + Coverage   53.06%   66.67%   +13.61%     
   ===========================================
     Files         489      493        +4     
     Lines       17314    29168    +11854     
     Branches     4482        0     -4482     
   ===========================================
   + Hits         9187    19447    +10260     
   - Misses       8127     9721     +1594     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | cypress | `?` | |
   | python | `66.67% <53.82%> (?)` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/13214?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [superset/constants.py](https://codecov.io/gh/apache/superset/pull/13214/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29uc3RhbnRzLnB5) | `100.00% <ø> (ø)` | |
   | [superset/examples/birth\_names.py](https://codecov.io/gh/apache/superset/pull/13214/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvYmlydGhfbmFtZXMucHk=) | `73.19% <ø> (ø)` | |
   | [...43f2fdb\_add\_granularity\_to\_charts\_where\_missing.py](https://codecov.io/gh/apache/superset/pull/13214/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbWlncmF0aW9ucy92ZXJzaW9ucy8wNzBjMDQzZjJmZGJfYWRkX2dyYW51bGFyaXR5X3RvX2NoYXJ0c193aGVyZV9taXNzaW5nLnB5) | `0.00% <0.00%> (ø)` | |
   | [...s/260bf0649a77\_migrate\_x\_dateunit\_in\_time\_range.py](https://codecov.io/gh/apache/superset/pull/13214/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbWlncmF0aW9ucy92ZXJzaW9ucy8yNjBiZjA2NDlhNzdfbWlncmF0ZV94X2RhdGV1bml0X2luX3RpbWVfcmFuZ2UucHk=) | `0.00% <0.00%> (ø)` | |
   | [...ons/versions/41ce8799acc3\_rename\_pie\_label\_type.py](https://codecov.io/gh/apache/superset/pull/13214/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbWlncmF0aW9ucy92ZXJzaW9ucy80MWNlODc5OWFjYzNfcmVuYW1lX3BpZV9sYWJlbF90eXBlLnB5) | `0.00% <0.00%> (ø)` | |
   | [superset/connectors/sqla/views.py](https://codecov.io/gh/apache/superset/pull/13214/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL3ZpZXdzLnB5) | `62.43% <4.34%> (ø)` | |
   | [superset/views/datasource.py](https://codecov.io/gh/apache/superset/pull/13214/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YXNvdXJjZS5weQ==) | `88.70% <16.66%> (ø)` | |
   | [superset/views/core.py](https://codecov.io/gh/apache/superset/pull/13214/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `72.91% <66.66%> (ø)` | |
   | [superset/charts/commands/exceptions.py](https://codecov.io/gh/apache/superset/pull/13214/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY2hhcnRzL2NvbW1hbmRzL2V4Y2VwdGlvbnMucHk=) | `92.85% <77.77%> (ø)` | |
   | [superset/db\_engine\_specs/elasticsearch.py](https://codecov.io/gh/apache/superset/pull/13214/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2VsYXN0aWNzZWFyY2gucHk=) | `90.24% <87.50%> (ø)` | |
   | ... and [938 more](https://codecov.io/gh/apache/superset/pull/13214/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/13214?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/13214?src=pr&el=footer). Last update [94893ff...c082af8](https://codecov.io/gh/apache/superset/pull/13214?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] codecov-io edited a comment on pull request #13214: feat(presto): add support for user impersonation

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #13214:
URL: https://github.com/apache/superset/pull/13214#issuecomment-782387935


   # [Codecov](https://codecov.io/gh/apache/superset/pull/13214?src=pr&el=h1) Report
   > Merging [#13214](https://codecov.io/gh/apache/superset/pull/13214?src=pr&el=desc) (4362f4f) into [master](https://codecov.io/gh/apache/superset/commit/2ce79823dfad61bce6196fcacd56a844f44818c0?el=desc) (2ce7982) will **increase** coverage by `13.56%`.
   > The diff coverage is `53.82%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/13214/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/superset/pull/13214?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff             @@
   ##           master   #13214       +/-   ##
   ===========================================
   + Coverage   53.06%   66.62%   +13.56%     
   ===========================================
     Files         489      493        +4     
     Lines       17314    29142    +11828     
     Branches     4482        0     -4482     
   ===========================================
   + Hits         9187    19415    +10228     
   - Misses       8127     9727     +1600     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | cypress | `?` | |
   | python | `66.62% <53.82%> (?)` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/13214?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [superset/constants.py](https://codecov.io/gh/apache/superset/pull/13214/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29uc3RhbnRzLnB5) | `100.00% <ø> (ø)` | |
   | [superset/examples/birth\_names.py](https://codecov.io/gh/apache/superset/pull/13214/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvYmlydGhfbmFtZXMucHk=) | `73.19% <ø> (ø)` | |
   | [...43f2fdb\_add\_granularity\_to\_charts\_where\_missing.py](https://codecov.io/gh/apache/superset/pull/13214/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbWlncmF0aW9ucy92ZXJzaW9ucy8wNzBjMDQzZjJmZGJfYWRkX2dyYW51bGFyaXR5X3RvX2NoYXJ0c193aGVyZV9taXNzaW5nLnB5) | `0.00% <0.00%> (ø)` | |
   | [...s/260bf0649a77\_migrate\_x\_dateunit\_in\_time\_range.py](https://codecov.io/gh/apache/superset/pull/13214/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbWlncmF0aW9ucy92ZXJzaW9ucy8yNjBiZjA2NDlhNzdfbWlncmF0ZV94X2RhdGV1bml0X2luX3RpbWVfcmFuZ2UucHk=) | `0.00% <0.00%> (ø)` | |
   | [...ons/versions/41ce8799acc3\_rename\_pie\_label\_type.py](https://codecov.io/gh/apache/superset/pull/13214/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbWlncmF0aW9ucy92ZXJzaW9ucy80MWNlODc5OWFjYzNfcmVuYW1lX3BpZV9sYWJlbF90eXBlLnB5) | `0.00% <0.00%> (ø)` | |
   | [superset/connectors/sqla/views.py](https://codecov.io/gh/apache/superset/pull/13214/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL3ZpZXdzLnB5) | `62.74% <4.34%> (ø)` | |
   | [superset/views/datasource.py](https://codecov.io/gh/apache/superset/pull/13214/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YXNvdXJjZS5weQ==) | `88.70% <16.66%> (ø)` | |
   | [superset/views/core.py](https://codecov.io/gh/apache/superset/pull/13214/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `71.80% <66.66%> (ø)` | |
   | [superset/charts/commands/exceptions.py](https://codecov.io/gh/apache/superset/pull/13214/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY2hhcnRzL2NvbW1hbmRzL2V4Y2VwdGlvbnMucHk=) | `92.85% <77.77%> (ø)` | |
   | [superset/db\_engine\_specs/elasticsearch.py](https://codecov.io/gh/apache/superset/pull/13214/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2VsYXN0aWNzZWFyY2gucHk=) | `90.24% <87.50%> (ø)` | |
   | ... and [938 more](https://codecov.io/gh/apache/superset/pull/13214/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/13214?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/13214?src=pr&el=footer). Last update [94893ff...4362f4f](https://codecov.io/gh/apache/superset/pull/13214?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] DRavikanth commented on pull request #13214: feat(presto): add support for user impersonation

Posted by GitBox <gi...@apache.org>.
DRavikanth commented on pull request #13214:
URL: https://github.com/apache/superset/pull/13214#issuecomment-832457726


   That would be great. I am on superset slack with user ID ravioravi. Can you
   ping me?
   
   Thanks,
   Ravi
   On Tue, May 4, 2021 at 23:52 rijojoseph07 ***@***.***> wrote:
   
   > @DRavikanth <https://github.com/DRavikanth> Can we connect over slack to
   > discuss this in detail? Please join superset slack channel.
   >
   > —
   > You are receiving this because you were mentioned.
   > Reply to this email directly, view it on GitHub
   > <https://github.com/apache/superset/pull/13214#issuecomment-832456060>,
   > or unsubscribe
   > <https://github.com/notifications/unsubscribe-auth/ACRGFYJB7Y5LPAQWUKJ37VTTMDTJVANCNFSM4X2W3ZIA>
   > .
   >
   


-- 
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] codecov-io edited a comment on pull request #13214: fix: Changes to support presto impersionation with ldap

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #13214:
URL: https://github.com/apache/superset/pull/13214#issuecomment-782387935


   # [Codecov](https://codecov.io/gh/apache/superset/pull/13214?src=pr&el=h1) Report
   > Merging [#13214](https://codecov.io/gh/apache/superset/pull/13214?src=pr&el=desc) (1b3f4fa) into [master](https://codecov.io/gh/apache/superset/commit/2ce79823dfad61bce6196fcacd56a844f44818c0?el=desc) (2ce7982) will **increase** coverage by `14.12%`.
   > The diff coverage is `52.36%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/13214/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/superset/pull/13214?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff             @@
   ##           master   #13214       +/-   ##
   ===========================================
   + Coverage   53.06%   67.18%   +14.12%     
   ===========================================
     Files         489      493        +4     
     Lines       17314    29097    +11783     
     Branches     4482        0     -4482     
   ===========================================
   + Hits         9187    19550    +10363     
   - Misses       8127     9547     +1420     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | cypress | `?` | |
   | python | `67.18% <52.36%> (?)` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/13214?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [superset/constants.py](https://codecov.io/gh/apache/superset/pull/13214/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29uc3RhbnRzLnB5) | `100.00% <ø> (ø)` | |
   | [superset/examples/birth\_names.py](https://codecov.io/gh/apache/superset/pull/13214/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvYmlydGhfbmFtZXMucHk=) | `73.19% <ø> (ø)` | |
   | [...43f2fdb\_add\_granularity\_to\_charts\_where\_missing.py](https://codecov.io/gh/apache/superset/pull/13214/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbWlncmF0aW9ucy92ZXJzaW9ucy8wNzBjMDQzZjJmZGJfYWRkX2dyYW51bGFyaXR5X3RvX2NoYXJ0c193aGVyZV9taXNzaW5nLnB5) | `0.00% <0.00%> (ø)` | |
   | [...s/260bf0649a77\_migrate\_x\_dateunit\_in\_time\_range.py](https://codecov.io/gh/apache/superset/pull/13214/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbWlncmF0aW9ucy92ZXJzaW9ucy8yNjBiZjA2NDlhNzdfbWlncmF0ZV94X2RhdGV1bml0X2luX3RpbWVfcmFuZ2UucHk=) | `0.00% <0.00%> (ø)` | |
   | [...ons/versions/41ce8799acc3\_rename\_pie\_label\_type.py](https://codecov.io/gh/apache/superset/pull/13214/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbWlncmF0aW9ucy92ZXJzaW9ucy80MWNlODc5OWFjYzNfcmVuYW1lX3BpZV9sYWJlbF90eXBlLnB5) | `0.00% <0.00%> (ø)` | |
   | [superset/connectors/sqla/views.py](https://codecov.io/gh/apache/superset/pull/13214/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL3ZpZXdzLnB5) | `62.43% <4.34%> (ø)` | |
   | [superset/views/datasource.py](https://codecov.io/gh/apache/superset/pull/13214/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YXNvdXJjZS5weQ==) | `88.70% <16.66%> (ø)` | |
   | [superset/viz.py](https://codecov.io/gh/apache/superset/pull/13214/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdml6LnB5) | `58.61% <27.27%> (ø)` | |
   | [superset/views/core.py](https://codecov.io/gh/apache/superset/pull/13214/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `72.91% <66.66%> (ø)` | |
   | [superset/charts/commands/exceptions.py](https://codecov.io/gh/apache/superset/pull/13214/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY2hhcnRzL2NvbW1hbmRzL2V4Y2VwdGlvbnMucHk=) | `92.85% <77.77%> (ø)` | |
   | ... and [937 more](https://codecov.io/gh/apache/superset/pull/13214/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/13214?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/13214?src=pr&el=footer). Last update [94893ff...1b3f4fa](https://codecov.io/gh/apache/superset/pull/13214?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] rijojoseph07 commented on pull request #13214: feat(presto): add support for user impersonation

Posted by GitBox <gi...@apache.org>.
rijojoseph07 commented on pull request #13214:
URL: https://github.com/apache/superset/pull/13214#issuecomment-831718900


   > Yes the admin credentials without impersonation works with no issues. I'm running Superset in debug mode and I don't see any logs associated with this. Thanks, Ravi
   > […](#)
   > On Mon, May 3, 2021 at 22:46 rijojoseph07 ***@***.***> wrote: Hi @DRavikanth <https://github.com/DRavikanth>, were you able to connect using admin credentials without impersonation? Also it will be helpful if you can run superset in debug mode and share the logs. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <[#13214 (comment)](https://github.com/apache/superset/pull/13214#issuecomment-831696748)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACRGFYILCSOHKHUGPSLITI3TL6C2PANCNFSM4X2W3ZIA> .
   
   @DRavikanth I just test this feature with superset 1.1.0 and presto 347 and it is working fine. Can you please check if impersonation is happening working as expected from the presto side via cli? 
   
   My connection URL looks like this : presto://presto-dns:443/hive/dev
   
   Extra : 
   
   {
           "metadata_params": {},
           "cost_estimate_enabled": true,
           "engine_params": {
              "connect_args":{
                 "protocol": "https",
                 "username":"admin",
                 "password":"<password>"
              }
            },
           "metadata_cache_timeout": {},
           "schemas_allowed_for_csv_upload": ["dev"]
       }
   
   I use ranger with presto, and have enabled impersonation from ranger.  You can also check presto server logs to see if your request is reaching presto. 
   


-- 
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] rijojoseph07 commented on pull request #13214: feat(presto): add support for user impersonation

Posted by GitBox <gi...@apache.org>.
rijojoseph07 commented on pull request #13214:
URL: https://github.com/apache/superset/pull/13214#issuecomment-831696748


   Hi @DRavikanth, were you able to connect using admin credentials without impersonation? Also it will be helpful if you can run superset in debug mode and share the logs.


-- 
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] codecov-io edited a comment on pull request #13214: feat(presto): add support for user impersonation

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #13214:
URL: https://github.com/apache/superset/pull/13214#issuecomment-782387935


   # [Codecov](https://codecov.io/gh/apache/superset/pull/13214?src=pr&el=h1) Report
   > Merging [#13214](https://codecov.io/gh/apache/superset/pull/13214?src=pr&el=desc) (4362f4f) into [master](https://codecov.io/gh/apache/superset/commit/2ce79823dfad61bce6196fcacd56a844f44818c0?el=desc) (2ce7982) will **increase** coverage by `13.58%`.
   > The diff coverage is `53.82%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/13214/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/superset/pull/13214?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff             @@
   ##           master   #13214       +/-   ##
   ===========================================
   + Coverage   53.06%   66.65%   +13.58%     
   ===========================================
     Files         489      493        +4     
     Lines       17314    29170    +11856     
     Branches     4482        0     -4482     
   ===========================================
   + Hits         9187    19442    +10255     
   - Misses       8127     9728     +1601     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | cypress | `?` | |
   | python | `66.65% <53.82%> (?)` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/13214?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [superset/constants.py](https://codecov.io/gh/apache/superset/pull/13214/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29uc3RhbnRzLnB5) | `100.00% <ø> (ø)` | |
   | [superset/examples/birth\_names.py](https://codecov.io/gh/apache/superset/pull/13214/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvYmlydGhfbmFtZXMucHk=) | `73.19% <ø> (ø)` | |
   | [...43f2fdb\_add\_granularity\_to\_charts\_where\_missing.py](https://codecov.io/gh/apache/superset/pull/13214/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbWlncmF0aW9ucy92ZXJzaW9ucy8wNzBjMDQzZjJmZGJfYWRkX2dyYW51bGFyaXR5X3RvX2NoYXJ0c193aGVyZV9taXNzaW5nLnB5) | `0.00% <0.00%> (ø)` | |
   | [...s/260bf0649a77\_migrate\_x\_dateunit\_in\_time\_range.py](https://codecov.io/gh/apache/superset/pull/13214/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbWlncmF0aW9ucy92ZXJzaW9ucy8yNjBiZjA2NDlhNzdfbWlncmF0ZV94X2RhdGV1bml0X2luX3RpbWVfcmFuZ2UucHk=) | `0.00% <0.00%> (ø)` | |
   | [...ons/versions/41ce8799acc3\_rename\_pie\_label\_type.py](https://codecov.io/gh/apache/superset/pull/13214/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbWlncmF0aW9ucy92ZXJzaW9ucy80MWNlODc5OWFjYzNfcmVuYW1lX3BpZV9sYWJlbF90eXBlLnB5) | `0.00% <0.00%> (ø)` | |
   | [superset/connectors/sqla/views.py](https://codecov.io/gh/apache/superset/pull/13214/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL3ZpZXdzLnB5) | `62.43% <4.34%> (ø)` | |
   | [superset/views/datasource.py](https://codecov.io/gh/apache/superset/pull/13214/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YXNvdXJjZS5weQ==) | `88.70% <16.66%> (ø)` | |
   | [superset/views/core.py](https://codecov.io/gh/apache/superset/pull/13214/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `71.80% <66.66%> (ø)` | |
   | [superset/charts/commands/exceptions.py](https://codecov.io/gh/apache/superset/pull/13214/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY2hhcnRzL2NvbW1hbmRzL2V4Y2VwdGlvbnMucHk=) | `92.85% <77.77%> (ø)` | |
   | [superset/db\_engine\_specs/elasticsearch.py](https://codecov.io/gh/apache/superset/pull/13214/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2VsYXN0aWNzZWFyY2gucHk=) | `90.24% <87.50%> (ø)` | |
   | ... and [938 more](https://codecov.io/gh/apache/superset/pull/13214/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/13214?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/13214?src=pr&el=footer). Last update [94893ff...4362f4f](https://codecov.io/gh/apache/superset/pull/13214?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org