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

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

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



##########
File path: superset/connectors/base/models.py
##########
@@ -62,14 +62,26 @@ class BaseDatasource(
     # ---------------------------------------------------------------
     __tablename__: Optional[str] = None  # {connector_name}_datasource
     baselink: Optional[str] = None  # url portion pointing to ModelView endpoint
-    column_class: Optional[Type] = None  # link to derivative of BaseColumn
-    metric_class: Optional[Type] = None  # link to derivative of BaseMetric
+
+    @property
+    def column_class(self) -> Type:

Review comment:
       This is done to ensure that the `column_class` etc. is not-optional.

##########
File path: superset/views/core.py
##########
@@ -965,28 +996,27 @@ def filter(self, datasource_type, datasource_id, column):
         return json_success(payload)
 
     @staticmethod
-    def remove_extra_filters(filters):
+    def remove_extra_filters(filters: List[Dict[str, Any]]) -> List[Dict[str, Any]]:
         """Extra filters are ones inherited from the dashboard's temporary context
         Those should not be saved when saving the chart"""
         return [f for f in filters if not f.get("isExtra")]
 
     def save_or_overwrite_slice(
         self,
-        args,
-        slc,
-        slice_add_perm,
-        slice_overwrite_perm,
-        slice_download_perm,
-        datasource_id,
-        datasource_type,
-        datasource_name,
-    ):
+        slc: Slice,
+        slice_add_perm: bool,
+        slice_overwrite_perm: bool,
+        slice_download_perm: bool,
+        datasource_id: int,
+        datasource_type: str,
+        datasource_name: str,
+    ) -> FlaskResponse:
         """Save or overwrite a slice"""
-        slice_name = args.get("slice_name")
-        action = args.get("action")
+        slice_name = request.args.get("slice_name")

Review comment:
       No need to parse `args` which was merely `request.args` when `request` is global.

##########
File path: superset/views/core.py
##########
@@ -2071,9 +2094,6 @@ def sqllab_viz(self):
         cols = []
         for config in data.get("columns"):
             column_name = config.get("name")
-            SqlaTable = ConnectorRegistry.sources["table"]

Review comment:
       This abstraction seemed necessary given we're restricting the models to the SQLA connector. 

##########
File path: superset/views/core.py
##########
@@ -228,19 +238,20 @@ def check_slice_perms(self, slice_id):
 
     form_data, slc = get_form_data(slice_id, use_slice_data=True)
 
-    viz_obj = get_viz(
-        datasource_type=slc.datasource.type,
-        datasource_id=slc.datasource.id,
-        form_data=form_data,
-        force=False,
-    )
+    if slc:

Review comment:
       There's a number of places where `slc` could be `None`.




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