ofekisr commented on a change in pull request #16991:
URL: https://github.com/apache/superset/pull/16991#discussion_r759095783
##########
File path: superset/security/manager.py
##########
@@ -1047,17 +1048,23 @@ def raise_for_access(
assert datasource
- if not (
- self.can_access_schema(datasource)
- or self.can_access("datasource_access", datasource.perm or "")
- or (
- feature_flag_manager.is_feature_enabled("DASHBOARD_RBAC")
- and self.can_access_based_on_dashboard(datasource)
- )
- ):
- raise SupersetSecurityException(
- self.get_datasource_access_error_object(datasource)
- )
+ self.raise_when_there_is_no_access_to(datasource)
+
+ def raise_when_there_is_no_access_to(self, datasource: BaseDatasource) ->
None:
Review comment:
@etr2460
You gave a good reference, but I still think is an anti-pattern, and they
are wrong too.
but with that reference, it should be discussed in a different context not
in that PR
@betodealmeida
There is a conflict between two principles:
consistency vs you should always leave the code you have just modified
cleaner than it was, a good catch and currently, I don't have a good argument
or opinion on what principle is stronger.
I'll rename it
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
