jscheffl commented on code in PR #67229:
URL: https://github.com/apache/airflow/pull/67229#discussion_r3276716933
##########
providers/amazon/src/airflow/providers/amazon/aws/triggers/eks.py:
##########
@@ -160,6 +161,7 @@ def __init__(
last_log_time=last_log_time,
logging_interval=logging_interval,
trigger_kwargs=trigger_kwargs,
+ execution_deadline=execution_deadline,
Review Comment:
This adds a coupling between the AWS and CNCF-K8s providers which are
packaged into different distributions. If we keep it like this the AWS provider
would gain a required dependency of the next future K8s version being
available. This dependency would need to be added to pyproject.toml as `# use
next version`
##########
providers/cncf/kubernetes/src/airflow/providers/cncf/kubernetes/operators/pod.py:
##########
@@ -908,6 +909,21 @@ def invoke_defer_method(
trigger_start_time = datetime.datetime.now(tz=datetime.timezone.utc)
+ # Translate ``execution_timeout`` into an absolute deadline plumbed
into
+ # the trigger.
+ execution_deadline: float | None = None
+ defer_timeout: datetime.timedelta | None = None
+ if self.execution_timeout is not None and context is not None:
+ ti = context.get("ti")
+ ti_start_date = getattr(ti, "start_date", None)
Review Comment:
I think it does not need to be as-pessimistic as implemented here. If a
context is passed we can assume it is a valid context with a start date, no
need to use `getattr()`
```suggestion
ti_start_date = context["ti"].start_date
```
##########
providers/cncf/kubernetes/src/airflow/providers/cncf/kubernetes/triggers/pod.py:
##########
@@ -123,6 +127,7 @@ def __init__(
last_log_time: DateTime | None = None,
logging_interval: int | None = None,
trigger_kwargs: dict | None = None,
+ execution_deadline: float | None = None,
Review Comment:
Why do you use a float for this? Can we just take a int for this?
--
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]
