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 2022/08/11 06:53:34 UTC

[GitHub] [superset] jileeon opened a new pull request, #21053: feat: add role, permission, user APIs

jileeon opened a new pull request, #21053:
URL: https://github.com/apache/superset/pull/21053

   ### SUMMARY
   Integrate role, permission, user APIs on Flask-AppBuilder into Apache Superset
   See https://github.com/apache/superset/issues/21050
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   These APIs can be used on Apache Superset Web Service. See the screenshot below.
   ![스크린샷 2022-08-11 오후 2 59 57](https://user-images.githubusercontent.com/5166067/184073700-5915bf94-1ae6-493f-a1ba-861fb315e9e8.png)
   
   ### TESTING INSTRUCTIONS
   N/A
   
   ### ADDITIONAL INFORMATION
   - [x] Required feature flags: Apache Superset 2.0 with [Flask-AppBuilder 4.1.0](https://github.com/dpgaspar/Flask-AppBuilder/releases/tag/v4.1.0) or later versions
   - [x] Introduces new 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.

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

[GitHub] [superset] jileeon closed pull request #21053: feat(api): add role, permission, user APIs

Posted by GitBox <gi...@apache.org>.
jileeon closed pull request #21053: feat(api): add role, permission, user APIs
URL: https://github.com/apache/superset/pull/21053


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

[GitHub] [superset] mayurnewase commented on pull request #21053: feat(api): add role, permission, user APIs

Posted by GitBox <gi...@apache.org>.
mayurnewase commented on PR #21053:
URL: https://github.com/apache/superset/pull/21053#issuecomment-1216032090

   Sure no problem, happy to help.
   If this PR had helped add the flag in the config with little description on what it does would have helped anyone looking to use these apis, but its alright I will add it later.


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

[GitHub] [superset] mayurnewase commented on pull request #21053: feat(api): add role, permission, user APIs

Posted by GitBox <gi...@apache.org>.
mayurnewase commented on PR #21053:
URL: https://github.com/apache/superset/pull/21053#issuecomment-1213643547

   I think best way to enable these is just by enabling `FAB_ADD_SECURITY_API` flag in config.


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

[GitHub] [superset] jileeon commented on pull request #21053: feat(api): add role, permission, user APIs

Posted by GitBox <gi...@apache.org>.
jileeon commented on PR #21053:
URL: https://github.com/apache/superset/pull/21053#issuecomment-1216002296

   Hello, 
   
   I've changed the commit for @mayurnewase 's comment, and I set FAB_ADD_SECURITY_API flag to False by default.
   Because I found that FAB_ADD_SECURITY_API flag isn't set to True by default in Flask-AppBuilder.
   
   See changes and review this.
   
   Thanks & Regards
    


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

[GitHub] [superset] nytai commented on a diff in pull request #21053: feat(api): add role, permission, user APIs

Posted by GitBox <gi...@apache.org>.
nytai commented on code in PR #21053:
URL: https://github.com/apache/superset/pull/21053#discussion_r944806636


##########
superset/initialization/__init__.py:
##########
@@ -28,6 +28,8 @@
 from flask_compress import Compress
 from werkzeug.middleware.proxy_fix import ProxyFix
 
+from flask_appbuilder.security.sqla.apis import PermissionApi, PermissionViewMenuApi, \
+    RoleApi, UserApi, ViewMenuApi

Review Comment:
   nit:
   ```suggestion
   from flask_appbuilder.security.sqla.apis import (
       PermissionApi, 
       PermissionViewMenuApi,
       RoleApi, 
       UserApi, 
       ViewMenuApi,
   )
   ```



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

[GitHub] [superset] jileeon commented on pull request #21053: feat(api): add role, permission, user APIs

Posted by GitBox <gi...@apache.org>.
jileeon commented on PR #21053:
URL: https://github.com/apache/superset/pull/21053#issuecomment-1216027915

   @mayurnewase You are right. We can just enable it by setting FAB_ADD_SECURITY_API to True.
   
   So I think that we no longer need this PR by the way. Do you mind if I close this PR? 
   
   P.S. I used these APIs since Superset 1.4 by importing Flask-AppBuilder locally only for using them. So it happens like that. Sorry for taking your time.
   
   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.

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

[GitHub] [superset] mayurnewase commented on pull request #21053: feat(api): add role, permission, user APIs

Posted by GitBox <gi...@apache.org>.
mayurnewase commented on PR #21053:
URL: https://github.com/apache/superset/pull/21053#issuecomment-1216008434

   If you just enable the flag, you won't need to add those apis in superset.
   I think this PR should be just about adding that flag set to False in the config, and you override it locally to True.


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

[GitHub] [superset] jileeon commented on pull request #21053: feat(api): add role, permission, user APIs

Posted by GitBox <gi...@apache.org>.
jileeon commented on PR #21053:
URL: https://github.com/apache/superset/pull/21053#issuecomment-1216037022

   @mayurnewase, That's great. I've been thinking of you said "add the flag in the config with little description". and I'm glad to add this flag later for other Superset users. 
   
   I going to close it now.
   
   Have a nice day :) 
   
   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.

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