Pedrinhonitz commented on code in PR #67138:
URL: https://github.com/apache/airflow/pull/67138#discussion_r3262783964
##########
providers/standard/src/airflow/providers/standard/sensors/external_task.py:
##########
@@ -267,13 +287,25 @@ def __init__(
self.external_dates_filter: str | None = None
def _get_dttm_filter(self, context: Context) ->
Sequence[datetime.datetime]:
+ execution_date_value = self.execution_date
+
+ if execution_date_value is not None:
+ if isinstance(execution_date_value, datetime.datetime):
+ return [execution_date_value]
+
+ import pendulum
Review Comment:
Thank you for your contribution.
I observed the following points:
I believe that lazy importing in this context doesn't make much sense; the
gain is low and it deviates from the file's standard. After all,
`_get_dttm_filter` runs on every `poke`/`defer`.
Another problem related to the `pendulum`: I noticed that we can `import
timezone` through the SDK, and we even have classes imported from it in this
file. It's worth evaluating if this makes sense; it would be one less library
in the file.
Here's a possible suggestion regarding parsing using
`airflow.providers.common.compat.sdk`. Some operators already use this pattern.
Therefore, we could replace:
```python
import pendulum
...
return [pendulum.parse(execution_date_value)]
```
Put:
```python
from airflow.providers.common.compat.sdk import (
AirflowSkipException,
BaseOperatorLink,
BaseSensorOperator,
conf,
timezone,
)
...
return [timezone.parse(execution_date_value)]
```
--
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]
