You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "arnoldmr01 (via GitHub)" <gi...@apache.org> on 2026/02/20 22:10:19 UTC
[PR] temp [airflow]
arnoldmr01 opened a new pull request, #62259:
URL: https://github.com/apache/airflow/pull/62259
<!-- SPDX-License-Identifier: Apache-2.0
https://www.apache.org/licenses/LICENSE-2.0 -->
<!--
Thank you for contributing!
Please provide above a brief description of the changes made in this pull request.
Write a good git commit message following this guide: http://chris.beams.io/posts/git-commit/
Please make sure that your code changes are covered with tests.
And in case of new features or big changes remember to adjust the documentation.
Feel free to ping (in general) for the review if you do not see reaction for a few days
(72 Hours is the minimum reaction time you can expect from volunteers) - we sometimes miss notifications.
In case of an existing issue, reference it using one of the following:
* closes: #ISSUE
* related: #ISSUE
-->
---
##### Was generative AI tooling used to co-author this PR?
<!--
If generative AI tooling has been used in the process of authoring this PR, please
change below checkbox to `[X]` followed by the name of the tool, uncomment the "Generated-by".
-->
- [ ] Yes (please specify the tool below)
<!--
Generated-by: [Tool Name] following [the guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
-->
---
* Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)** for more information. Note: commit author/co-author name and email in commits become permanently public when merged.
* For fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
* When adding dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
* For significant user-facing changes create newsfragment: `{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] Add run_after to TriggerDagRunOperator [airflow]
Posted by "arnoldmr01 (via GitHub)" <gi...@apache.org>.
arnoldmr01 commented on code in PR #62259:
URL: https://github.com/apache/airflow/pull/62259#discussion_r2895305008
##########
task-sdk/src/airflow/sdk/api/client.py:
##########
@@ -694,12 +694,17 @@ def trigger(
run_id: str,
conf: dict | None = None,
logical_date: datetime | None = None,
+ run_after: datetime | None = None,
reset_dag_run: bool = False,
Review Comment:
Just for check. I might be mistaken something, but it seemed like attribute run_after is already existed. Do we still need to add migration version file?
--
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] Add run_after to TriggerDagRunOperator [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #62259:
URL: https://github.com/apache/airflow/pull/62259#issuecomment-4179459819
@arnoldmr01 This PR has been converted to **draft** because it does not yet meet our [Pull Request quality criteria](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-quality-criteria).
**Issues found:**
- :x: **Merge conflicts**: This PR has merge conflicts with the `main` branch. Your branch is 162 commits behind `main`. Please rebase your branch (`git fetch origin && git rebase origin/main`), resolve the conflicts, and push again. See [contributing quick start](https://github.com/apache/airflow/blob/main/contributing-docs/03a_contributors_quick_start_beginners.rst).
- :warning: **Unresolved review comments**: This PR has 3 unresolved review threads from maintainers: **@jason810496** (MEMBER): 3 unresolved threads. Please review and resolve all inline review comments before requesting another review. You can resolve a conversation by clicking 'Resolve conversation' on each thread after addressing the feedback. See [pull request guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst).
> **Note:** Your branch is **162 commits behind `main`**. Some check failures may be caused by changes in the base branch rather than by your PR. Please rebase your branch and push again to get up-to-date CI results.
**What to do next:**
- The comment informs you what you need to do.
- Fix each issue, then mark the PR as "Ready for review" in the GitHub UI - but only after making sure that all the issues are fixed.
- There is no rush — take your time and work at your own pace. We appreciate your contribution and are happy to wait for updates.
- Maintainers will then proceed with a normal review.
Converting a PR to draft is **not** a rejection — it is an invitation to bring the PR up to the project's standards so that maintainer review time is spent productively. There is no rush — take your time and work at your own pace. We appreciate your contribution and are happy to wait for updates. If you have questions, feel free to ask on the [Airflow Slack](https://s.apache.org/airflow-slack).
--
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] Add run_after to TriggerDagRunOperator [airflow]
Posted by "jason810496 (via GitHub)" <gi...@apache.org>.
jason810496 commented on code in PR #62259:
URL: https://github.com/apache/airflow/pull/62259#discussion_r3001090228
##########
providers/standard/src/airflow/providers/standard/operators/trigger_dagrun.py:
##########
@@ -453,3 +476,42 @@ def _trigger_dag_run_af_2_execute_complete(
f"{self.trigger_dag_id} return {state} which is not in {self.failed_states}"
f" or {self.allowed_states}"
)
+
+
+@overload
+def _validate_datetime_param(name: str, value: ArgNotSet) -> ArgNotSet: ...
+@overload
+def _validate_datetime_param(name: str, value: None) -> None: ...
+@overload
+def _validate_datetime_param(name: str, value: str) -> str: ...
+@overload
+def _validate_datetime_param(name: str, value: datetime.datetime) -> datetime.datetime: ...
+
+
+def _validate_datetime_param(
+ name: str,
+ value: object,
+) -> str | datetime.datetime | None | ArgNotSet:
+ if value is NOTSET:
+ return NOTSET
+ if value is None or isinstance(value, (str, datetime.datetime)):
Review Comment:
MyPy _should_ be able to respect the `NOTSET` if we change the input type annotation from `: object` to `str | datetime.datetime | None | ArgNotSet`
```python
def _validate_datetime_param(
name: str,
value: str | datetime.datetime | None | ArgNotSet,
) -> str | datetime.datetime | None | ArgNotSet:
```
Btw, the `is_arg_set` helper might be more appropriate to distinguish the `NOTSET`:
https://github.com/apache/airflow/blob/d22505da575fe06b825e999ea03cde2291abf5b5/airflow-core/src/airflow/serialization/definitions/notset.py#L39-L40
--
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] Add run_after to TriggerDagRunOperator [airflow]
Posted by "arnoldmr01 (via GitHub)" <gi...@apache.org>.
arnoldmr01 commented on code in PR #62259:
URL: https://github.com/apache/airflow/pull/62259#discussion_r3128158175
##########
providers/standard/src/airflow/providers/standard/operators/trigger_dagrun.py:
##########
@@ -162,6 +163,11 @@ class TriggerDagRunOperator(BaseOperator):
"wait_for_completion",
"skip_when_already_exists",
)
+
+ attributes_not_supported_in_airflow_2 = {
+ "run_after": NOTSET,
+ "note": None,
+ }
Review Comment:
Yes, I originally wanted to distinguish whether users explicitly passed run_after or not. But for the use case rn, it seems that we don't really need to distinguish NOTSET from None. If no precise datetime is provided for run_after, we either fall back to logical_date or pass None in the following flow.
--
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] Add run_after to TriggerDagRunOperator [airflow]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #62259:
URL: https://github.com/apache/airflow/pull/62259#discussion_r3066493297
##########
providers/standard/src/airflow/providers/standard/operators/trigger_dagrun.py:
##########
@@ -162,6 +163,11 @@ class TriggerDagRunOperator(BaseOperator):
"wait_for_completion",
"skip_when_already_exists",
)
+
+ attributes_not_suppported_in_airflow_2: Sequence[str] = (
+ "run_after",
+ "note",
+ )
Review Comment:
Typo in the new attribute name: `attributes_not_suppported_in_airflow_2` has an extra “p” (suppported). Renaming it to `attributes_not_supported_in_airflow_2` would improve readability and avoid propagating the typo to other references.
##########
airflow-core/src/airflow/api_fastapi/execution_api/versions/v2026_04_06.py:
##########
@@ -184,3 +187,13 @@ class AddDagEndpoint(VersionChange):
description = __doc__
instructions_to_migrate_to_previous_version = (endpoint("/dags/{dag_id}", ["GET"]).didnt_exist,)
+
+
+class AddRunAfterFiled(VersionChange):
+ """Add run_after parameter to TriggerDAGRunPayload Model."""
+
+ description = __doc__
+
+ instructions_to_migrate_to_previous_version = (
+ schema(TriggerDAGRunPayload).field("run_after").didnt_exist,
+ )
Review Comment:
The new version change class name has a typo: `AddRunAfterFiled` should likely be `AddRunAfterField` (and the corresponding import/registration updated). This will make the version history easier to read and search.
##########
task-sdk/src/airflow/sdk/api/client.py:
##########
@@ -711,12 +711,17 @@ def trigger(
run_id: str,
conf: dict | None = None,
logical_date: datetime | None = None,
+ run_after: datetime | None = None,
reset_dag_run: bool = False,
note: str | None = None,
) -> OKResponse | ErrorResponse:
"""Trigger a Dag run via the API server."""
body = TriggerDAGRunPayload(
- logical_date=logical_date, conf=conf or {}, reset_dag_run=reset_dag_run, note=note
+ logical_date=logical_date,
+ conf=conf or {},
+ reset_dag_run=reset_dag_run,
+ note=note,
+ run_after=run_after,
)
Review Comment:
`DagRunOperations.trigger()` now accepts `run_after` and includes it in `TriggerDAGRunPayload`, but there is no corresponding request-serialization test ensuring `run_after` is present in the POST body when provided (similar to the existing `test_trigger` assertions for `logical_date`). Adding a test case would prevent regressions in the client’s HTTP payload format.
##########
providers/standard/src/airflow/providers/standard/operators/trigger_dagrun.py:
##########
@@ -291,16 +303,27 @@ def _trigger_dag_af_3(self, context, run_id, parsed_logical_date):
deferrable=self.deferrable,
)
- if self.note and "note" in inspect.signature(DagRunTriggerException.__init__).parameters:
+ parameters = inspect.signature(DagRunTriggerException.__init__).parameters
+ if self.note and "note" in parameters:
kwargs_accepted["note"] = self.note
+ if parsed_run_after and "run_after" in parameters:
+ kwargs_accepted["run_after"] = parsed_run_after
+
raise DagRunTriggerException(**kwargs_accepted)
def _trigger_dag_af_2(self, context, run_id, parsed_logical_date):
try:
- if self.note:
- self.log.warning("Parameter 'note' is not supported in Airflow 2.x and will be ignored.")
-
+ unsupported_parameters = [
+ attr
+ for attr in self.attributes_not_suppported_in_airflow_2
+ if getattr(self, attr, NOTSET) is not NOTSET or None
Review Comment:
In the Airflow 2.x path, the unsupported-parameter detection is incorrect: `getattr(self, attr, NOTSET) is not NOTSET or None` will treat `None` as “set”, so `note` (default None) will be reported as unsupported even when the user did not pass it. Consider checking for “arg is set” explicitly (e.g., `val is not NOTSET and val is not None`) so warnings only fire when the parameter is actually provided.
--
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] Add run_after to TriggerDagRunOperator [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #62259:
URL: https://github.com/apache/airflow/pull/62259#issuecomment-4043270731
@arnoldmr01 This PR has been converted to **draft** because it does not yet meet our [Pull Request quality criteria](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-quality-criteria).
**Issues found:**
- :x: **Other failing CI checks**: Failing: Additional PROD image tests / Airflow CTL integration tests with PROD image. Run `prek run --from-ref main` locally to reproduce. See [static checks docs](https://github.com/apache/airflow/blob/main/contributing-docs/08_static_code_checks.rst).
- :warning: **Unresolved review comments**: This PR has 5 unresolved review threads from maintainers. Please review and resolve all inline review comments before requesting another review. You can resolve a conversation by clicking 'Resolve conversation' on each thread after addressing the feedback. See [pull request guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst).
> **Note:** Your branch is **544 commits behind `main`**. Some check failures may be caused by changes in the base branch rather than by your PR. Please rebase your branch and push again to get up-to-date CI results.
**What to do next:**
- The comment informs you what you need to do.
- Fix each issue, then mark the PR as "Ready for review" in the GitHub UI - but only after making sure that all the issues are fixed.
- Maintainers will then proceed with a normal review.
Converting a PR to draft is **not** a rejection — it is an invitation to bring the PR up to the project's standards so that maintainer review time is spent productively. If you have questions, feel free to ask on the [Airflow Slack](https://s.apache.org/airflow-slack).
--
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] Add run_after to TriggerDagRunOperator [airflow]
Posted by "arnoldmr01 (via GitHub)" <gi...@apache.org>.
arnoldmr01 commented on PR #62259:
URL: https://github.com/apache/airflow/pull/62259#issuecomment-4320868476
Might be unrelated CI failure
--
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] Add run_after to TriggerDagRunOperator [airflow]
Posted by "ZhaoMJ (via GitHub)" <gi...@apache.org>.
ZhaoMJ commented on PR #62259:
URL: https://github.com/apache/airflow/pull/62259#issuecomment-4321150169
@arnoldmr01 I just realized I've also implemented this in https://github.com/apache/airflow/pull/65856
Besides Airflow 2 compat, there are mainly some differences in default value handling. Would you mind taking a look?
--
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] Add run_after to TriggerDagRunOperator [airflow]
Posted by "arnoldmr01 (via GitHub)" <gi...@apache.org>.
arnoldmr01 commented on code in PR #62259:
URL: https://github.com/apache/airflow/pull/62259#discussion_r2895318751
##########
providers/standard/src/airflow/providers/standard/operators/trigger_dagrun.py:
##########
@@ -291,16 +316,23 @@ def _trigger_dag_af_3(self, context, run_id, parsed_logical_date):
deferrable=self.deferrable,
)
- if self.note and "note" in inspect.signature(DagRunTriggerException.__init__).parameters:
+ parameters = inspect.signature(DagRunTriggerException.__init__).parameters
+ if self.note and "note" in parameters:
kwargs_accepted["note"] = self.note
+ if parsed_run_after and "run_after" in parameters:
+ kwargs_accepted["run_after"] = parsed_run_after
+
raise DagRunTriggerException(**kwargs_accepted)
def _trigger_dag_af_2(self, context, run_id, parsed_logical_date):
try:
if self.note:
self.log.warning("Parameter 'note' is not supported in Airflow 2.x and will be ignored.")
+ if self.run_after is not NOTSET:
+ self.log.warning("Parameter 'run_after' is not supported in Airflow 2.x and will be ignored.")
Review Comment:
Thanks for your advices, I will take it and modify it.
--
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] Add run_after to TriggerDagRunOperator [airflow]
Posted by "jason810496 (via GitHub)" <gi...@apache.org>.
jason810496 commented on PR #62259:
URL: https://github.com/apache/airflow/pull/62259#issuecomment-4344238361
Thanks @arnoldmr01 for focusing on this. Nice feature.
--
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] Add run_after to TriggerDagRunOperator [airflow]
Posted by "jason810496 (via GitHub)" <gi...@apache.org>.
jason810496 commented on code in PR #62259:
URL: https://github.com/apache/airflow/pull/62259#discussion_r2894785172
##########
providers/standard/src/airflow/providers/standard/operators/trigger_dagrun.py:
##########
@@ -291,16 +316,23 @@ def _trigger_dag_af_3(self, context, run_id, parsed_logical_date):
deferrable=self.deferrable,
)
- if self.note and "note" in inspect.signature(DagRunTriggerException.__init__).parameters:
+ parameters = inspect.signature(DagRunTriggerException.__init__).parameters
+ if self.note and "note" in parameters:
kwargs_accepted["note"] = self.note
+ if parsed_run_after and "run_after" in parameters:
+ kwargs_accepted["run_after"] = parsed_run_after
+
raise DagRunTriggerException(**kwargs_accepted)
def _trigger_dag_af_2(self, context, run_id, parsed_logical_date):
try:
if self.note:
self.log.warning("Parameter 'note' is not supported in Airflow 2.x and will be ignored.")
+ if self.run_after is not NOTSET:
Review Comment:
Correct! I just double check on whether we support `run_after` in AF2.
The `DagRun.run_after` was introduced in `3.0.0`
https://github.com/apache/airflow/blob/243fe86d4b3e59bb12977b3e36ca3f2ed27ca0f8/airflow-core/src/airflow/migrations/versions/0058_3_0_0_add_dagrun_run_after.py#L18-L62
and the AF2 REST API didn't support `run_after`, but it support `execution_date` (the `logical_date` in AF3).
https://github.com/apache/airflow/blob/243fe86d4b3e59bb12977b3e36ca3f2ed27ca0f8/airflow-core/src/airflow/migrations/versions/0058_3_0_0_add_dagrun_run_after.py#L18-L62
##########
providers/standard/src/airflow/providers/standard/operators/trigger_dagrun.py:
##########
@@ -215,18 +217,38 @@ def __init__(
f"Expected str, datetime.datetime, or None for parameter 'logical_date'. Got {type(logical_date).__name__}"
)
+ self.run_after = run_after
+ if run_after is NOTSET:
+ self.run_after = NOTSET
+ elif run_after is None or isinstance(run_after, (str, datetime.datetime)):
+ self.run_after = run_after
+ else:
+ raise TypeError(
+ f"Expected str, datetime.datetime, or None for parameter 'run_after'. Got {type(run_after).__name__}"
+ )
Review Comment:
How about adding a method to handle the common logic between `run_after` and `logical_date`?
##########
providers/standard/src/airflow/providers/standard/operators/trigger_dagrun.py:
##########
@@ -291,16 +316,23 @@ def _trigger_dag_af_3(self, context, run_id, parsed_logical_date):
deferrable=self.deferrable,
)
- if self.note and "note" in inspect.signature(DagRunTriggerException.__init__).parameters:
+ parameters = inspect.signature(DagRunTriggerException.__init__).parameters
+ if self.note and "note" in parameters:
kwargs_accepted["note"] = self.note
+ if parsed_run_after and "run_after" in parameters:
+ kwargs_accepted["run_after"] = parsed_run_after
+
raise DagRunTriggerException(**kwargs_accepted)
def _trigger_dag_af_2(self, context, run_id, parsed_logical_date):
try:
if self.note:
self.log.warning("Parameter 'note' is not supported in Airflow 2.x and will be ignored.")
+ if self.run_after is not NOTSET:
+ self.log.warning("Parameter 'run_after' is not supported in Airflow 2.x and will be ignored.")
Review Comment:
How about adding attribute like `attrs_not_allowed_in_af2` (perhaps better naming) for better maintainability as we might add more attributes for `TriggerDagRunOperator` in the long term?
##########
task-sdk/src/airflow/sdk/api/client.py:
##########
@@ -694,12 +694,17 @@ def trigger(
run_id: str,
conf: dict | None = None,
logical_date: datetime | None = None,
+ run_after: datetime | None = None,
reset_dag_run: bool = False,
Review Comment:
We need to add exection API migration version file like the last adding `note` one.
##########
providers/standard/src/airflow/providers/standard/operators/trigger_dagrun.py:
##########
@@ -215,18 +217,38 @@ def __init__(
f"Expected str, datetime.datetime, or None for parameter 'logical_date'. Got {type(logical_date).__name__}"
)
+ self.run_after = run_after
+ if run_after is NOTSET:
+ self.run_after = NOTSET
+ elif run_after is None or isinstance(run_after, (str, datetime.datetime)):
+ self.run_after = run_after
+ else:
+ raise TypeError(
+ f"Expected str, datetime.datetime, or None for parameter 'run_after'. Got {type(run_after).__name__}"
+ )
+
if fail_when_dag_is_paused and AIRFLOW_V_3_0_PLUS:
raise NotImplementedError("Setting `fail_when_dag_is_paused` not yet supported for Airflow 3.x")
def execute(self, context: Context):
if self.logical_date is NOTSET:
- # If no logical_date is provided we will set utcnow()
- parsed_logical_date = timezone.utcnow()
+ if self.run_after is not NOTSET:
+ parsed_logical_date = None
+ else:
+ # If no logical_date is provided we will set utcnow()
+ parsed_logical_date = timezone.utcnow()
elif self.logical_date is None or isinstance(self.logical_date, datetime.datetime):
parsed_logical_date = self.logical_date # type: ignore
elif isinstance(self.logical_date, str):
parsed_logical_date = timezone.parse(self.logical_date)
+ if self.run_after is NOTSET:
+ parsed_run_after = parsed_logical_date
+ elif self.run_after is None or isinstance(self.run_after, datetime.datetime):
+ parsed_run_after = self.run_after # type: ignore
+ elif isinstance(self.run_after, str):
+ parsed_run_after = timezone.parse(self.run_after)
Review Comment:
It seems also worthwhile to have a common utils to handle the type across `None | datetime | str` for both `run_after` and `logical_date`.
--
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] Add run_after to TriggerDagRunOperator [airflow]
Posted by "arnoldmr01 (via GitHub)" <gi...@apache.org>.
arnoldmr01 commented on code in PR #62259:
URL: https://github.com/apache/airflow/pull/62259#discussion_r2999199055
##########
providers/standard/src/airflow/providers/standard/operators/trigger_dagrun.py:
##########
@@ -453,3 +470,20 @@ def _trigger_dag_run_af_2_execute_complete(
f"{self.trigger_dag_id} return {state} which is not in {self.failed_states}"
f" or {self.allowed_states}"
)
+
+
+def _validate_datetime_param(name: str, value):
+ if value is NOTSET:
+ return NOTSET
+ if value is None or isinstance(value, (str, datetime.datetime)):
+ return value
+ raise TypeError(
+ f"Expected str, datetime.datetime, or None for parameter '{name}'. Got {type(value).__name__}"
+ )
+
+
+def _parse_datetime_param(value: str | datetime.datetime | None):
Review Comment:
The way we handle the case where logical_date is NOTSET depends on self.run_after, so I did not handle ArgNotSet in _parse_datetime_param. And the reason why I used cast is that mypy cannot filter ArgNotSet.
--
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] Add run_after to TriggerDagRunOperator [airflow]
Posted by "arnoldmr01 (via GitHub)" <gi...@apache.org>.
arnoldmr01 commented on code in PR #62259:
URL: https://github.com/apache/airflow/pull/62259#discussion_r3080693275
##########
providers/standard/src/airflow/providers/standard/operators/trigger_dagrun.py:
##########
@@ -162,6 +163,11 @@ class TriggerDagRunOperator(BaseOperator):
"wait_for_completion",
"skip_when_already_exists",
)
+
+ attributes_not_suppported_in_airflow_2: Sequence[str] = (
+ "run_after",
+ "note",
+ )
Review Comment:
Modified attributes_not_supported_in_airflow_2 to
```
attributes_not_supported_in_airflow_2 = { "run_after": NOTSET, "note": None, }
```
and Modified the typo
--
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] Add run_after to TriggerDagRunOperator [airflow]
Posted by "arnoldmr01 (via GitHub)" <gi...@apache.org>.
arnoldmr01 commented on code in PR #62259:
URL: https://github.com/apache/airflow/pull/62259#discussion_r2895333290
##########
providers/standard/src/airflow/providers/standard/operators/trigger_dagrun.py:
##########
@@ -215,18 +217,38 @@ def __init__(
f"Expected str, datetime.datetime, or None for parameter 'logical_date'. Got {type(logical_date).__name__}"
)
+ self.run_after = run_after
+ if run_after is NOTSET:
+ self.run_after = NOTSET
+ elif run_after is None or isinstance(run_after, (str, datetime.datetime)):
+ self.run_after = run_after
+ else:
+ raise TypeError(
+ f"Expected str, datetime.datetime, or None for parameter 'run_after'. Got {type(run_after).__name__}"
+ )
+
if fail_when_dag_is_paused and AIRFLOW_V_3_0_PLUS:
raise NotImplementedError("Setting `fail_when_dag_is_paused` not yet supported for Airflow 3.x")
def execute(self, context: Context):
if self.logical_date is NOTSET:
- # If no logical_date is provided we will set utcnow()
- parsed_logical_date = timezone.utcnow()
+ if self.run_after is not NOTSET:
+ parsed_logical_date = None
+ else:
+ # If no logical_date is provided we will set utcnow()
+ parsed_logical_date = timezone.utcnow()
elif self.logical_date is None or isinstance(self.logical_date, datetime.datetime):
parsed_logical_date = self.logical_date # type: ignore
elif isinstance(self.logical_date, str):
parsed_logical_date = timezone.parse(self.logical_date)
+ if self.run_after is NOTSET:
+ parsed_run_after = parsed_logical_date
+ elif self.run_after is None or isinstance(self.run_after, datetime.datetime):
+ parsed_run_after = self.run_after # type: ignore
+ elif isinstance(self.run_after, str):
+ parsed_run_after = timezone.parse(self.run_after)
Review Comment:
Will modify it.
--
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] Add run_after to TriggerDagRunOperator [airflow]
Posted by "arnoldmr01 (via GitHub)" <gi...@apache.org>.
arnoldmr01 commented on code in PR #62259:
URL: https://github.com/apache/airflow/pull/62259#discussion_r3080698645
##########
airflow-core/src/airflow/api_fastapi/execution_api/versions/v2026_04_06.py:
##########
@@ -184,3 +187,13 @@ class AddDagEndpoint(VersionChange):
description = __doc__
instructions_to_migrate_to_previous_version = (endpoint("/dags/{dag_id}", ["GET"]).didnt_exist,)
+
+
+class AddRunAfterFiled(VersionChange):
+ """Add run_after parameter to TriggerDAGRunPayload Model."""
+
+ description = __doc__
+
+ instructions_to_migrate_to_previous_version = (
+ schema(TriggerDAGRunPayload).field("run_after").didnt_exist,
+ )
Review Comment:
Changed AddRunAfterFiled to AddRunAfterField
--
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] Add run_after to TriggerDagRunOperator [airflow]
Posted by "arnoldmr01 (via GitHub)" <gi...@apache.org>.
arnoldmr01 commented on code in PR #62259:
URL: https://github.com/apache/airflow/pull/62259#discussion_r2999283989
##########
providers/standard/src/airflow/providers/standard/operators/trigger_dagrun.py:
##########
@@ -291,16 +303,21 @@ def _trigger_dag_af_3(self, context, run_id, parsed_logical_date):
deferrable=self.deferrable,
)
- if self.note and "note" in inspect.signature(DagRunTriggerException.__init__).parameters:
+ parameters = inspect.signature(DagRunTriggerException.__init__).parameters
+ if self.note and "note" in parameters:
kwargs_accepted["note"] = self.note
+ if parsed_run_after and "run_after" in parameters:
+ kwargs_accepted["run_after"] = parsed_run_after
+
raise DagRunTriggerException(**kwargs_accepted)
def _trigger_dag_af_2(self, context, run_id, parsed_logical_date):
try:
- if self.note:
- self.log.warning("Parameter 'note' is not supported in Airflow 2.x and will be ignored.")
-
+ self.log.warning(
Review Comment:
The defaults of `note` and `run_after` are set be None and NOTSET, so `hasattr` will always return true.
--
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] Add run_after to TriggerDagRunOperator [airflow]
Posted by "arnoldmr01 (via GitHub)" <gi...@apache.org>.
arnoldmr01 commented on code in PR #62259:
URL: https://github.com/apache/airflow/pull/62259#discussion_r2999283989
##########
providers/standard/src/airflow/providers/standard/operators/trigger_dagrun.py:
##########
@@ -291,16 +303,21 @@ def _trigger_dag_af_3(self, context, run_id, parsed_logical_date):
deferrable=self.deferrable,
)
- if self.note and "note" in inspect.signature(DagRunTriggerException.__init__).parameters:
+ parameters = inspect.signature(DagRunTriggerException.__init__).parameters
+ if self.note and "note" in parameters:
kwargs_accepted["note"] = self.note
+ if parsed_run_after and "run_after" in parameters:
+ kwargs_accepted["run_after"] = parsed_run_after
+
raise DagRunTriggerException(**kwargs_accepted)
def _trigger_dag_af_2(self, context, run_id, parsed_logical_date):
try:
- if self.note:
- self.log.warning("Parameter 'note' is not supported in Airflow 2.x and will be ignored.")
-
+ self.log.warning(
Review Comment:
The defaults of `note` and `run_after` are set be None and NOTSET, so `hasattr` is always return true.
--
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] Add run_after to TriggerDagRunOperator [airflow]
Posted by "arnoldmr01 (via GitHub)" <gi...@apache.org>.
arnoldmr01 commented on code in PR #62259:
URL: https://github.com/apache/airflow/pull/62259#discussion_r2895331318
##########
providers/standard/src/airflow/providers/standard/operators/trigger_dagrun.py:
##########
@@ -215,18 +217,38 @@ def __init__(
f"Expected str, datetime.datetime, or None for parameter 'logical_date'. Got {type(logical_date).__name__}"
)
+ self.run_after = run_after
+ if run_after is NOTSET:
+ self.run_after = NOTSET
+ elif run_after is None or isinstance(run_after, (str, datetime.datetime)):
+ self.run_after = run_after
+ else:
+ raise TypeError(
+ f"Expected str, datetime.datetime, or None for parameter 'run_after'. Got {type(run_after).__name__}"
+ )
Review Comment:
Yas, this part should be extract as a func to handle those same logic. I will modify it.
--
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] Add run_after to TriggerDagRunOperator [airflow]
Posted by "jason810496 (via GitHub)" <gi...@apache.org>.
jason810496 commented on code in PR #62259:
URL: https://github.com/apache/airflow/pull/62259#discussion_r2999056060
##########
providers/standard/src/airflow/providers/standard/operators/trigger_dagrun.py:
##########
@@ -453,3 +470,20 @@ def _trigger_dag_run_af_2_execute_complete(
f"{self.trigger_dag_id} return {state} which is not in {self.failed_states}"
f" or {self.allowed_states}"
)
+
+
+def _validate_datetime_param(name: str, value):
+ if value is NOTSET:
+ return NOTSET
+ if value is None or isinstance(value, (str, datetime.datetime)):
+ return value
+ raise TypeError(
+ f"Expected str, datetime.datetime, or None for parameter '{name}'. Got {type(value).__name__}"
+ )
+
+
+def _parse_datetime_param(value: str | datetime.datetime | None):
Review Comment:
We're missing the return type annotation here for both _validate_datetime_param and _parse_datetime_param. After we add them, we should be able to remove the cast("str | datetime.datetime | None" ... above.
Additionally, we might need to add `@overload` to make mypy aware of the return type accurately.
##########
providers/standard/src/airflow/providers/standard/operators/trigger_dagrun.py:
##########
@@ -291,16 +303,21 @@ def _trigger_dag_af_3(self, context, run_id, parsed_logical_date):
deferrable=self.deferrable,
)
- if self.note and "note" in inspect.signature(DagRunTriggerException.__init__).parameters:
+ parameters = inspect.signature(DagRunTriggerException.__init__).parameters
+ if self.note and "note" in parameters:
kwargs_accepted["note"] = self.note
+ if parsed_run_after and "run_after" in parameters:
+ kwargs_accepted["run_after"] = parsed_run_after
+
raise DagRunTriggerException(**kwargs_accepted)
def _trigger_dag_af_2(self, context, run_id, parsed_logical_date):
try:
- if self.note:
- self.log.warning("Parameter 'note' is not supported in Airflow 2.x and will be ignored.")
-
+ self.log.warning(
Review Comment:
Would it be better to log only if any of `self.attributes_not_suppported_in_airflow_2` being set via `hasattr`?
--
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] Add run_after to TriggerDagRunOperator [airflow]
Posted by "arnoldmr01 (via GitHub)" <gi...@apache.org>.
arnoldmr01 commented on code in PR #62259:
URL: https://github.com/apache/airflow/pull/62259#discussion_r3080683165
##########
providers/standard/src/airflow/providers/standard/operators/trigger_dagrun.py:
##########
@@ -291,16 +303,27 @@ def _trigger_dag_af_3(self, context, run_id, parsed_logical_date):
deferrable=self.deferrable,
)
- if self.note and "note" in inspect.signature(DagRunTriggerException.__init__).parameters:
+ parameters = inspect.signature(DagRunTriggerException.__init__).parameters
+ if self.note and "note" in parameters:
kwargs_accepted["note"] = self.note
+ if parsed_run_after and "run_after" in parameters:
+ kwargs_accepted["run_after"] = parsed_run_after
+
raise DagRunTriggerException(**kwargs_accepted)
def _trigger_dag_af_2(self, context, run_id, parsed_logical_date):
try:
- if self.note:
- self.log.warning("Parameter 'note' is not supported in Airflow 2.x and will be ignored.")
-
+ unsupported_parameters = [
+ attr
+ for attr in self.attributes_not_suppported_in_airflow_2
+ if getattr(self, attr, NOTSET) is not NOTSET or None
Review Comment:
Modified attributes_not_supported_in_airflow_2 as a set. e.g.
`attributes_not_supported_in_airflow_2 = {
"run_after": NOTSET,
"note": None,
}
`
The keys the not supported attributes and the values are the default values.
The logic to check if those not supported attributes are set by the user is
```
unsupported_parameters = []
for attr, default_value in self.attributes_not_suppported_in_airflow_2.items():
value = getattr(self, attr, default_value)
if value is not default_value:
unsupported_parameters.append(attr)
```
--
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] Add run_after to TriggerDagRunOperator [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #62259:
URL: https://github.com/apache/airflow/pull/62259#issuecomment-4300899037
@arnoldmr01 This PR has been converted to **draft** because it does not yet meet our [Pull Request quality criteria](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-quality-criteria).
**Issues found:**
- :x: **Merge conflicts**: this branch has conflicts with `main` and cannot be merged as-is. Rebase: `git fetch upstream main && git rebase upstream/main`, resolve, force-push.
- :x: **Unresolved review comments** (1 thread (from maintainers)): please walk through each unresolved review thread. **Even if a suggestion looks incorrect or irrelevant** — and some of them will be, especially any comments left by automated reviewers like GitHub Copilot — it is still the author's responsibility to respond: apply the fix, reply in-thread with a brief explanation of why the suggestion does not apply, or resolve the thread if the feedback is no longer relevant. **Once you believe a thread is resolved — whether by pushing a fix or by explaining why the suggestion doesn't apply — please mark it as resolved yourself by clicking the 'Resolve conversation' button at the bottom of each thread.** Reviewers don't auto-close their own threads, so an addressed-but-unresolved thread reads as 'still waiting on the author' and keeps the PR from moving forward.
**What to do next:**
- Address the items above.
- Make sure static checks pass locally: `prek run --from-ref main --stage pre-commit`.
- Mark the PR as "Ready for review" when you are done.
Converting a PR to draft is **not** a rejection — it is an invitation to bring the PR up to the project's standards so that maintainer review time is spent productively. There is no rush — take your time and work at your own pace. We appreciate your contribution and are happy to wait for updates. If you have questions, feel free to ask on the [Airflow Slack](https://s.apache.org/airflow-slack).
--
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] Add run_after to TriggerDagRunOperator [airflow]
Posted by "arnoldmr01 (via GitHub)" <gi...@apache.org>.
arnoldmr01 commented on code in PR #62259:
URL: https://github.com/apache/airflow/pull/62259#discussion_r3080693275
##########
providers/standard/src/airflow/providers/standard/operators/trigger_dagrun.py:
##########
@@ -162,6 +163,11 @@ class TriggerDagRunOperator(BaseOperator):
"wait_for_completion",
"skip_when_already_exists",
)
+
+ attributes_not_suppported_in_airflow_2: Sequence[str] = (
+ "run_after",
+ "note",
+ )
Review Comment:
Modified attributes_not_supported_in_airflow_2 to
```
attributes_not_supported_in_airflow_2 = { "run_after": NOTSET, "note": None, }
```
--
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] Add run_after to TriggerDagRunOperator [airflow]
Posted by "jason810496 (via GitHub)" <gi...@apache.org>.
jason810496 merged PR #62259:
URL: https://github.com/apache/airflow/pull/62259
--
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] Add run_after to TriggerDagRunOperator [airflow]
Posted by "jason810496 (via GitHub)" <gi...@apache.org>.
jason810496 commented on code in PR #62259:
URL: https://github.com/apache/airflow/pull/62259#discussion_r3108046649
##########
providers/standard/src/airflow/providers/standard/operators/trigger_dagrun.py:
##########
@@ -162,6 +163,11 @@ class TriggerDagRunOperator(BaseOperator):
"wait_for_completion",
"skip_when_already_exists",
)
+
+ attributes_not_supported_in_airflow_2 = {
+ "run_after": NOTSET,
+ "note": None,
+ }
Review Comment:
Do we intentionally set different default value here?
```
attributes_not_supported_in_airflow_2 = {
"run_after": NOTSET,
"note": None,
}
```
--
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] Add run_after to TriggerDagRunOperator [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #62259:
URL: https://github.com/apache/airflow/pull/62259#issuecomment-4323637150
@arnoldmr01 — There are 1 unresolved review thread on this PR from @jason810496. Could you either push a fix or reply in each thread explaining why the feedback doesn't apply? Once you believe the feedback is addressed, mark the thread as resolved so the reviewer isn't re-pinged needlessly. Thanks!
---
_Note: This comment was drafted by an AI-assisted triage tool and may contain mistakes. Once you have addressed the points above, an Apache Airflow maintainer — a real person — will take the next look at your PR. We use this [two-stage triage process](https://github.com/apache/airflow/blob/main/contributing-docs/25_maintainer_pr_triage.md#why-the-first-pass-is-automated) so that our maintainers' limited time is spent where it matters most: the conversation with you._
--
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] Add run_after to TriggerDagRunOperator [airflow]
Posted by "jason810496 (via GitHub)" <gi...@apache.org>.
jason810496 commented on code in PR #62259:
URL: https://github.com/apache/airflow/pull/62259#discussion_r3038636329
##########
uv.lock:
##########
@@ -2645,7 +2645,7 @@ docs = [
[package.metadata]
requires-dist = [
- { name = "aiobotocore", marker = "extra == 'aiobotocore'", specifier = ">=3.0.0" },
+ { name = "aiobotocore", extras = ["boto3"], marker = "extra == 'aiobotocore'", specifier = ">=2.26.0" },
{ name = "apache-airflow", editable = "." },
Review Comment:
Even though the CI passed, I'm still concerned about the `uv.lock` change. Would it be possible to revert the `uv.lock` change before we merge?
--
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