vandonr-amz commented on code in PR #29001:
URL: https://github.com/apache/airflow/pull/29001#discussion_r1072990846
##########
airflow/providers/amazon/aws/sensors/batch.py:
##########
@@ -74,16 +73,13 @@ def poke(self, context: Context) -> bool:
raise AirflowException(f"Batch sensor failed. Unknown AWS Batch job
status: {state}")
Review Comment:
I was thinking about it, there is the "every change is a breaking change"
way of thinking, but also I don't see why anyone would go out of their way to
get the hook from a sensor outside of that sensor...
##########
airflow/providers/amazon/aws/sensors/ec2.py:
##########
@@ -62,8 +63,11 @@ def __init__(
self.aws_conn_id = aws_conn_id
self.region_name = region_name
+ @cached_property
+ def hook(self):
+ return EC2Hook(aws_conn_id=self.aws_conn_id,
region_name=self.region_name)
Review Comment:
I'm usually in favor of less lines of code for more density on one screen
##########
airflow/providers/amazon/aws/sensors/eks.py:
##########
@@ -98,13 +99,15 @@ def __init__(
self.region = region
super().__init__(**kwargs)
- def poke(self, context: Context):
- eks_hook = EksHook(
+ @cached_property
+ def hook(self):
+ return EksHook(
aws_conn_id=self.aws_conn_id,
region_name=self.region,
)
Review Comment:
If there was already a base sensor, yeah, sure. To add it just for this,
idk...
##########
airflow/providers/amazon/aws/sensors/glue.py:
##########
@@ -61,10 +62,13 @@ def __init__(
self.errored_states: list[str] = ["FAILED", "STOPPED", "TIMEOUT"]
self.next_log_token: str | None = None
+ @cached_property
+ def hook(self):
+ return GlueJobHook(aws_conn_id=self.aws_conn_id)
Review Comment:
I mostly kept the existing behavior. I don't know what is a real use case
for passing the region, maybe that depends on the operator/sensor ?
##########
airflow/providers/amazon/aws/sensors/sqs.py:
##########
@@ -152,7 +151,7 @@ def poke(self, context: Context):
:param context: the context object
:return: ``True`` if message is available or ``False``
"""
- sqs_conn = self.get_hook().get_conn()
+ sqs_conn = self.hook.get_conn()
Review Comment:
it'd break the tests, I'll take a closer look tomorrow
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
