dabla commented on code in PR #73928:
URL: https://github.com/apache/airflow/pull/73928#discussion_r4205339383
##########
providers/standard/tests/unit/standard/sensors/test_date_time.py:
##########
@@ -156,19 +193,37 @@ def
test_async_start_from_trigger_localizes_naive_datetime(self):
start_from_trigger=True,
dag=self.dag,
)
- assert op.start_trigger_args.trigger_kwargs["moment"] ==
pendulum.datetime(2020, 1, 1, tz="UTC")
+ if AIRFLOW_V_3_3_PLUS:
+ assert op.start_trigger_args.trigger_kwargs["target_time"] ==
datetime.datetime(2020, 1, 1, 0, 0)
Review Comment:
**[warning]** This assertion is unchanged: the test still does not check
that the naive `datetime` is localized.
Now that the trigger handles a `datetime` `target_time`, the test can check
the resolved value as its name promises, for example:
```python
trigger = DateTimeTrigger(**op.start_trigger_args.trigger_kwargs)
assert trigger.moment == pendulum.datetime(2020, 1, 1, tz="UTC")
```
##########
providers/standard/src/airflow/providers/standard/triggers/temporal.py:
##########
@@ -31,31 +31,57 @@ class DateTimeTrigger(BaseTrigger):
"""
Trigger based on a datetime.
- A trigger that fires exactly once, at the given datetime, give or take
- a few seconds.
-
- The provided datetime MUST be in UTC.
+ Pass either ``moment`` (a tz-aware datetime) or ``target_time`` (a string,
possibly a Jinja
+ template, or a datetime). With ``start_from_trigger``, the operator lists
``target_time`` in its
+ own ``template_fields`` and puts it in
``start_trigger_args.trigger_kwargs``, so the triggerer
+ renders it in place before ``run()``; it is then parsed into ``moment`` on
first use.
:param moment: when to yield event
+ :param target_time: raw (possibly templated) datetime string, an
alternative to ``moment``
:param end_from_trigger: whether the trigger should mark the task
successful after time condition
reached or resume the task after time condition reached.
"""
- def __init__(self, moment: datetime.datetime, *, end_from_trigger: bool =
False) -> None:
+ def __init__(
+ self,
+ moment: datetime.datetime | None = None,
+ *,
+ target_time: datetime.datetime | str | None = None,
+ end_from_trigger: bool = False,
+ ) -> None:
super().__init__()
- if not isinstance(moment, datetime.datetime):
- raise TypeError(f"Expected datetime.datetime type for moment. Got
{type(moment)}")
- # Make sure it's in UTC
- if moment.tzinfo is None:
- raise ValueError("You cannot pass naive datetimes")
- self.moment: pendulum.DateTime = timezone.convert_to_utc(moment)
+ if (moment is None) == (target_time is None):
+ raise TypeError("DateTimeTrigger requires exactly one of 'moment'
or 'target_time'")
+ self.target_time = target_time
+ self._moment: pendulum.DateTime | None = None
+ if moment is not None:
+ if not isinstance(moment, datetime.datetime):
+ raise TypeError(f"Expected datetime.datetime type for moment.
Got {type(moment)}")
+ # Make sure it's in UTC
+ if moment.tzinfo is None:
+ raise ValueError("You cannot pass naive datetimes")
+ self._moment = timezone.convert_to_utc(moment)
self.end_from_trigger = end_from_trigger
+ @property
+ def moment(self) -> pendulum.DateTime:
+ if self._moment is None:
+ # Resolved lazily: by now the triggerer has rendered target_time
in place.
+ target_time: Any = self.target_time
+ if isinstance(target_time, datetime.datetime):
Review Comment:
**[warning]** The `datetime` handling is fixed, but `DateTimeTrigger` still
has no tests for `target_time`.
Thanks, `moment` now mirrors `DateTimeSensor._moment`, so a `datetime`
`target_time` no longer fails at run time. What is still missing is the test
part of that remark: `triggers/test_temporal.py` is untouched and does not
mention `target_time`. Please add tests there for
- `target_time` as `str`, as an aware `datetime` and as a naive `datetime`
(localized to UTC),
- the "exactly one of `moment` / `target_time`" validation in `__init__`,
- the `serialize()` round trip when the trigger was built from `target_time`.
--
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]
