You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "vatsrahul1001 (via GitHub)" <gi...@apache.org> on 2025/04/08 18:39:56 UTC
[PR] Make default as time.now() for TriggerDagRunOperator [airflow]
vatsrahul1001 opened a new pull request, #48969:
URL: https://github.com/apache/airflow/pull/48969
closes: https://github.com/apache/airflow/issues/48412
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)** for more information.
In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [airflow-core/newsfragments](https://github.com/apache/airflow/tree/main/airflow-core/newsfragments).
--
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] Make default as time.now() for TriggerDagRunOperator [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #48969:
URL: https://github.com/apache/airflow/pull/48969#discussion_r2034865852
##########
providers/standard/src/airflow/providers/standard/operators/trigger_dagrun.py:
##########
@@ -190,10 +190,12 @@ def __init__(
self.logical_date = logical_date
def execute(self, context: Context):
- if self.logical_date is None or isinstance(self.logical_date, datetime.datetime):
+ if isinstance(self.logical_date, datetime.datetime):
parsed_logical_date = self.logical_date
- else:
+ elif isinstance(self.logical_date, str):
parsed_logical_date = timezone.parse(self.logical_date)
+ else:
+ parsed_logical_date = timezone.utcnow()
Review Comment:
This removes the possibility to explicitly supply None as the logical date. We need an extra marker, something like
```
def __init__(self, ..., logical_date: str | datetime | None | ArgNotSet = NOT_SET):
...
def execute(self, context):
if self.logical_date is None or isinstance(self.logical_date, datetime):
parsed_logical_date = self.logical_date
elif isinstance(self.logical_date, str):
parsed_logical_date = timezone.parse(self.logical_date)
else:
parsed_logical_date = utcnow()
```
--
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] Make default as time.now() for TriggerDagRunOperator [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #48969:
URL: https://github.com/apache/airflow/pull/48969#discussion_r2034477878
##########
providers/standard/src/airflow/providers/standard/operators/trigger_dagrun.py:
##########
@@ -190,10 +190,12 @@ def __init__(
self.logical_date = logical_date
def execute(self, context: Context):
- if self.logical_date is None or isinstance(self.logical_date, datetime.datetime):
+ if isinstance(self.logical_date, datetime.datetime):
parsed_logical_date = self.logical_date
- else:
+ elif isinstance(self.logical_date, str):
parsed_logical_date = timezone.parse(self.logical_date)
+ else:
+ parsed_logical_date = timezone.utcnow()
Review Comment:
Can we please add a comment why we do 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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Make default as time.now() for TriggerDagRunOperator [airflow]
Posted by "kaxil (via GitHub)" <gi...@apache.org>.
kaxil commented on code in PR #48969:
URL: https://github.com/apache/airflow/pull/48969#discussion_r2035388504
##########
providers/standard/src/airflow/providers/standard/operators/trigger_dagrun.py:
##########
@@ -180,19 +180,23 @@ def __init__(
self.failed_states = [DagRunState.FAILED]
self.skip_when_already_exists = skip_when_already_exists
self._defer = deferrable
-
- if logical_date is not None and not isinstance(logical_date, (str, datetime.datetime)):
- type_name = type(logical_date).__name__
+ self.logical_date = logical_date
Review Comment:
This line is redundant
--
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] Make default as time.now() for TriggerDagRunOperator [airflow]
Posted by "kaxil (via GitHub)" <gi...@apache.org>.
kaxil commented on code in PR #48969:
URL: https://github.com/apache/airflow/pull/48969#discussion_r2035391003
##########
providers/standard/src/airflow/providers/standard/operators/trigger_dagrun.py:
##########
@@ -180,19 +180,23 @@ def __init__(
self.failed_states = [DagRunState.FAILED]
self.skip_when_already_exists = skip_when_already_exists
self._defer = deferrable
-
- if logical_date is not None and not isinstance(logical_date, (str, datetime.datetime)):
- type_name = type(logical_date).__name__
+ self.logical_date = logical_date
Review Comment:
Can handle it separately -- it is a nit anyway
--
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] Make default as time.now() for TriggerDagRunOperator [airflow]
Posted by "kaxil (via GitHub)" <gi...@apache.org>.
kaxil merged PR #48969:
URL: https://github.com/apache/airflow/pull/48969
--
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