[GitHub] [incubator-superset] john-bodley commented on a change in pull request #9920: style(mypy): Enforcing typing for views.database

[GitBox <gi...@apache.org>]

john-bodley commented on a change in pull request #9920:
URL: https://github.com/apache/incubator-superset/pull/9920#discussion_r430768557



##########
File path: superset/views/database/filters.py
##########
@@ -14,24 +14,26 @@
 # KIND, either express or implied.  See the License for the
 # specific language governing permissions and limitations
 # under the License.
+from typing import Any, Optional, Set
+
 from sqlalchemy import or_
+from sqlalchemy.orm import Query
 
 from superset import security_manager
 from superset.views.base import BaseFilter
 
 
 class DatabaseFilter(BaseFilter):
     # TODO(bogdan): consider caching.
-    def schema_access_databases(self):  # noqa pylint: disable=no-self-use
-        found_databases = set()
-        for vm in security_manager.user_view_menu_names("schema_access"):
-            database_name, _ = security_manager.unpack_schema_perm(vm)
-            found_databases.add(database_name)
-        return found_databases
+    def schema_access_databases(self) -> Set[str]:  # noqa pylint: disable=no-self-use
+        return {
+            security_manager.unpack_schema_perm(vm)[0]
+            for vm in security_manager.user_view_menu_names("schema_access")
+        }
 
-    def apply(
-        self, query, func
-    ):  # noqa pylint: disable=unused-argument,arguments-differ
+    def apply(  # noqa pylint: disable=unused-argument
+        self, query: Query, value: Optional[Any]

Review comment:
       I renamed `func` to `value` for consistency with the base class.

##########
File path: superset/views/database/mixins.py
##########
@@ -214,23 +215,23 @@ def _pre_add_update(self, database):
                 "schema_access", security_manager.get_schema_perm(database, schema)
             )
 
-    def pre_add(self, database):
+    def pre_add(self, database: Database) -> None:
         self._pre_add_update(database)
 
-    def pre_update(self, database):
+    def pre_update(self, database: Database) -> None:
         self._pre_add_update(database)
 
-    def pre_delete(self, obj):  # pylint: disable=no-self-use
-        if obj.tables:
+    def pre_delete(self, database: Database) -> None:  # pylint: disable=no-self-use

Review comment:
       I renamed `obj` to `database` for consistency.




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