ferruzzi commented on code in PR #29053: URL: https://github.com/apache/airflow/pull/29053#discussion_r1086965963
########## airflow/providers/amazon/aws/sensors/eks.py: ########## @@ -57,7 +58,71 @@ ) -class EksClusterStateSensor(BaseSensorOperator): +class EksBaseSensor(BaseSensorOperator): + """ + Base class to check various EKS states. + Subclasses need to implement get_state and get_terminal_states methods. + + :param cluster_name: The name of the Cluster + :param target_state: Will return successfully when that state is reached. + :param target_state_type: The enum containing the states, + will be used to convert the target state if it has to be converted from a string + :param aws_conn_id: The Airflow connection used for AWS credentials. + If this is None or empty then the default boto3 behaviour is used. If + running Airflow in a distributed manner and aws_conn_id is None or + empty, then the default boto3 configuration would be used (and must be + maintained on each worker node). + :param region: Which AWS region the connection should use. + If this is None or empty then the default boto3 behaviour is used. + """ + + def __init__( + self, + *, + cluster_name: str, + target_state: Any, Review Comment: I think I would prefer to see this as ```suggestion target_state: ClusterStates | NodegroupStates | FargateProfileStates, ``` and modify the return type hinting on the get_state() methods below, but I can also see how that would be getting a bit too verbose, so I'll leave it up to you. Maybe a new Type: ``` from typing import NewType TargetStates = NewType('TargetStates', ClusterStates | NodegroupStates | FargateProfileStates) ``` here or in the hook? Just a thought (non-blocking) -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org