[GitHub] [airflow] mlgruby commented on a change in pull request #11531: Add type hints to aws provider

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

mlgruby commented on a change in pull request #11531:
URL: https://github.com/apache/airflow/pull/11531#discussion_r508807981



##########
File path: airflow/providers/amazon/aws/hooks/sagemaker.py
##########
@@ -918,7 +912,9 @@ def list_processing_jobs(self, **kwargs) -> List[Dict]:  # noqa: D402
         )
         return results
 
-    def _list_request(self, partial_func, result_key: str, max_results: Optional[int] = None) -> List[Dict]:
+    def _list_request(
+        self, partial_func: Callable, result_key: str, max_results: Optional[int] = None
+    ) -> List[Dict]:

Review comment:
       ```suggestion
       ) -> List[dict]:
   ```
   if we are not specifying types for keys and values then let's keep it simple with `List[dict]`

##########
File path: airflow/providers/amazon/aws/sensors/cloud_formation.py
##########
@@ -86,8 +95,10 @@ def poke(self, context):
             return False
         raise ValueError(f'Stack {self.stack_name} in bad state: {stack_status}')
 
-    def get_hook(self):
+    def get_hook(self) -> AWSCloudFormationHook:
         """Create and return an AWSCloudFormationHook"""
-        if not self.hook:
-            self.hook = AWSCloudFormationHook(aws_conn_id=self.aws_conn_id, region_name=self.region_name)
+        if self.hook:

Review comment:
       This `if` is because of `None` default value of `hook` and hence mypy complaining for it? 

##########
File path: airflow/providers/amazon/aws/operators/sagemaker_base.py
##########
@@ -41,12 +43,11 @@ class SageMakerBaseOperator(BaseOperator):
     integer_fields = []  # type: Iterable[Iterable[str]]
 
     @apply_defaults
-    def __init__(self, *, config, aws_conn_id='aws_default', **kwargs):
+    def __init__(self, *, config: dict, aws_conn_id: str = 'aws_default', **kwargs):
         super().__init__(**kwargs)
 
         self.aws_conn_id = aws_conn_id
         self.config = config
-        self.hook = None

Review comment:
       Well, I liked this one :) 




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