You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "hussein-awala (via GitHub)" <gi...@apache.org> on 2023/11/30 21:26:25 UTC
[PR] Avoid creating the hook in the EmrServerlessCancelJobsTrigger init [airflow]
hussein-awala opened a new pull request, #35992:
URL: https://github.com/apache/airflow/pull/35992
This might be considered a breaking change, but it's completely useless to create the hook in the init method because we create an instance from the trigger in the operator when we defer it, which means an extra needed DB query to fetch the secrets.
The original reason for this implementation was accessing `JOB_INTERMEDIATE_STATES` list, which is a class param, so no need to create an instance from the class to get it.
cc: @eladkal
--
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
Re: [PR] Avoid creating the hook in the EmrServerlessCancelJobsTrigger init [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on PR #35992:
URL: https://github.com/apache/airflow/pull/35992#issuecomment-1835447437
> This might be considered a breaking change, but it's completely useless to create the hook in the init method because we create an instance from the trigger in the operator when we defer it, which means an extra needed DB query to fetch the secrets.
I dont think this qualify as breaking change
--
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
Re: [PR] Avoid creating the hook in the EmrServerlessCancelJobsTrigger init [airflow]
Posted by "hussein-awala (via GitHub)" <gi...@apache.org>.
hussein-awala merged PR #35992:
URL: https://github.com/apache/airflow/pull/35992
--
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
Re: [PR] Avoid creating the hook in the EmrServerlessCancelJobsTrigger init [airflow]
Posted by "hussein-awala (via GitHub)" <gi...@apache.org>.
hussein-awala commented on code in PR #35992:
URL: https://github.com/apache/airflow/pull/35992#discussion_r1411382275
##########
airflow/providers/amazon/aws/triggers/emr.py:
##########
@@ -461,4 +460,9 @@ def __init__(
)
def hook(self) -> AwsGenericHook:
- return self.hook_instance
+ return EmrServerlessHook(self.aws_conn_id)
+
+ @property
+ def hook_instance(self) -> AwsGenericHook:
+ """This property is added for backward compatibility."""
+ return self.hook()
Review Comment:
I added `hook_instance` as a property, so it will be available when the user tries to access it, but it's lazy evaluated and created only when accessing it.
That makes the change backward compatible.
--
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