You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2022/10/09 18:45:43 UTC

[GitHub] [airflow] Taragolis commented on a diff in pull request #26949: Add type hints for EMR hook

Taragolis commented on code in PR #26949:
URL: https://github.com/apache/airflow/pull/26949#discussion_r990824364


##########
airflow/providers/amazon/aws/hooks/emr.py:
##########
@@ -20,14 +20,22 @@
 import json
 import warnings
 from time import sleep
-from typing import Any, Callable
+from typing import TYPE_CHECKING, Any, Callable
 
 from botocore.exceptions import ClientError
 
 from airflow.compat.functools import cached_property
 from airflow.exceptions import AirflowException, AirflowNotFoundException
 from airflow.providers.amazon.aws.hooks.base_aws import AwsBaseHook
 
+if TYPE_CHECKING:
+    from mypy_boto3_emr.literals import ClusterStateType
+    from mypy_boto3_emr.type_defs import (
+        ListClustersOutputTypeDef,
+        RunJobFlowInputRequestTypeDef,
+        RunJobFlowOutputTypeDef,
+    )
+
 
 class EmrHook(AwsBaseHook):

Review Comment:
   Also suggestion change base class for EmrHook to the same way as it done for RdsHook:
   https://github.com/apache/airflow/blob/7efdeed5eccbf5cb709af40c8c66757e59c957ed/airflow/providers/amazon/aws/hooks/rds.py#L23-L29
   
   It would allow get type annotation to `EmrHook.conn` property and `EmrHook.get_conn()` method



##########
airflow/providers/amazon/aws/hooks/emr.py:
##########
@@ -118,13 +128,13 @@ def create_job_flow(self, job_flow_overrides: dict[str, Any]) -> dict[str, Any]:
                         stacklevel=2,
                     )
                 config = emr_conn.extra_dejson.copy()
-        config.update(job_flow_overrides)
+        config.update(**job_flow_overrides)

Review Comment:
   I thought IDE warn because it decide that `RunJobFlowInputRequestTypeDef` not suitable for `dict.update`.
   
   One thing: `job_flow_overrides` might not be `RunJobFlowInputRequestTypeDef`. It is more related to how EMR hook deal with this parameter - described in recent PR https://github.com/apache/airflow/pull/26687



-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org