mistercrunch commented on issue #4565: Move access permissions methods to security manager URL: https://github.com/apache/incubator-superset/pull/4565#issuecomment-375127580 Why 2 security managers? To me the inheritance scheme should be `SupersetSecurityManager` (new abstraction) inherits from FAB's `SecurityManager`, and environment-specific say `AirbnbSupersetSecurityManager` should be forced to derived `SupersetSecurityManager`. I don't think it makes sense to grow in the direction of having many security managers. What's next? `DatabaseSecurityManager`, `DashboardSecurityManager`? The scheme I described is a [necessary] breaking change since I'd assume in most environment have a custom SecurityManager that currently derives FAB's and will need to change to derive Superset's. An [perhaps compatible] alternative would be a `SupersetSecurityManagerMixin` that we'd somehow inject into FAB's or the enviornment-specific `SecurityManger`. Not sure if runtime injection of mixin is an acceptable pattern though.
---------------------------------------------------------------- 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: [email protected] With regards, Apache Git Services
