You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "bhardwajparth51 (via GitHub)" <gi...@apache.org> on 2026/10/19 23:41:11 UTC
[PR] Fix mark-state include_past/future on runs without logical_date (#75134) [airflow]
bhardwajparth51 opened a new pull request, #75170:
URL: https://github.com/apache/airflow/pull/75170
When setting task instance or task group state on a DAG run that has no `logical_date` (manual or asset-triggered runs), passing `include_past` or `include_future` caused downstream task clearing to cross run boundaries. This inadvertently cleared failed tasks in other runs of the same DAG and could push them back into queued state.
Since past and future runs are undefined without a `logical_date`, the UI already disables these toggles and the clear-task API rejects them with a 400. This PR brings the mark-state routes to parity by rejecting the combination upfront with HTTP 400 instead of executing cross-run clears.
### What changed
- **API validation**: Added early validation for single TI and task group PATCH / dry-run requests before locking or DB mutation.
- **Bulk handling**: Validates entities upfront using a cached DagRun lookup per run to avoid N+1 queries. Invalid entities are recorded in bulk errors and filtered out of `valid_entities`, so they cannot participate in run locking or expand downstream selections in regional or non-regional paths.
- **Model guard**: `SerializedDAG.set_task_instance_state` and `set_task_group_state` validate this condition and raise `ValueError` before clearing downstream tasks, which the API maps to a 400.
- Shared canonical validation helper and error message across API and model layers.
### Breaking change note
This is only breaking for clients sending `include_past` or `include_future` on runs without a `logical_date`. Those requests previously returned 200 while silently corrupting task states across unrelated runs; they now properly fail with 400.
### Test plan
- Single TI on manual run + `include_past`/`future` returns 400; tasks in dated run remain intact.
- Single TI on manual run without flags updates normally (200).
- Single TI on dated run + `include_future` succeeds (200).
- Dry-run TI and task group return 400 when flags are set on a run without `logical_date`.
- Bulk requests with invalid entities return 200 with per-item 400 errors and do not clear other runs.
- Bulk requests with mixed valid and invalid entities (including duplicate entries for the same task) apply valid updates, reject invalid ones, and leave other runs untouched.
- Unit tests verify `SerializedDAG` raises `ValueError` for both single TI and task group state setters.
closes: #75134
---
##### Was generative AI tooling used to co-author this PR?
- [X] Yes
<!--
-->
--
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] fix: mark state include_past/future on runs without logical_date [airflow]
Posted by "luc-pimentel (via GitHub)" <gi...@apache.org>.
luc-pimentel commented on code in PR #75170:
URL: https://github.com/apache/airflow/pull/75170#discussion_r4407618059
##########
airflow-core/src/airflow/api_fastapi/core_api/services/public/task_instances.py:
##########
@@ -668,6 +709,22 @@ def _categorize_entities(
)
continue
+ if isinstance(entity, BulkTaskInstanceBody) and (entity.include_past or entity.include_future):
+ run_key = (dag_id, dag_run_id)
+ if run_key not in dag_run_cache:
+ dag_run_cache[run_key] = self.session.scalar(
+ select(DagRun).where(DagRun.dag_id == dag_id, DagRun.run_id == dag_run_id)
+ )
+ dag_run = dag_run_cache[run_key]
+ if dag_run is not None:
+ try:
+ validate_past_future_for_logical_date(
+ entity.include_past, entity.include_future, dag_run.logical_date
+ )
+ except ValueError as err:
+ results.errors.append({"error": str(err), "status_code": status.HTTP_400_BAD_REQUEST})
Review Comment:
`_categorize_entities` returns the no-logical-date error before checking access to the Dag. Should we move this validation below the authorization check?
With a user outside the Dag team, I get a per-item 400 for an existing dateless run but 403 for dated or missing runs. On the base revision, all three return 403. This exposes run information to a caller who cannot access that Dag.
---
Drafted-by: OpenCode 1.19.0 (GPT-6 Astra); reviewed by @luc-pimentel before posting
##########
airflow-core/tests/unit/models/test_dag.py:
##########
@@ -3739,6 +3739,49 @@ def get_ti_from_db(task):
}
+@pytest.mark.parametrize("flag", ["future", "past"])
+def test_set_task_instance_state_no_logical_date_raises_value_error(session, dag_maker, flag):
+ """Test that set_task_instance_state with past/future raises ValueError on a run with no logical_date."""
+ from airflow.serialization.definitions.dag import NO_LOGICAL_DATE_PAST_FUTURE_ERROR
+
+ with dag_maker("test_set_ti_no_logical_date", session=session, serialized=True) as dag:
+ EmptyOperator(task_id="task_1") >> EmptyOperator(task_id="task_2")
+
+ dagrun = dag_maker.create_dagrun(run_id="manual", run_type=DagRunType.MANUAL, logical_date=None)
+
+ with pytest.raises(ValueError, match=re.escape(NO_LOGICAL_DATE_PAST_FUTURE_ERROR)):
+ dag.set_task_instance_state(
+ task_id="task_1",
+ run_id=dagrun.run_id,
+ state=State.SUCCESS,
+ session=session,
+ **{flag: True},
+ )
Review Comment:
This test checks the exception but not whether the task changed first. Should we snapshot the task states before the call, then compare them after flushing and reloading the session?
before = {ti.task_id: ti.state for ti in dagrun.get_task_instances(session=session)}
# Existing call and ValueError assertion go here.
session.flush()
session.expire_all()
assert {ti.task_id: ti.state for ti in dagrun.get_task_instances(session=session)} == before
Moving the guard below `set_state` leaves all 18 new tests passing, although task_1 becomes success. The state-preservation assertion fails for both flags on that mutation and passes for both on this head.
---
Drafted-by: OpenCode 1.19.0 (GPT-6 Astra); reviewed by @luc-pimentel before posting
--
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] Fix mark-state include_past/future on runs without logical_date (#75134) [airflow]
Posted by "boring-cyborg[bot] (via GitHub)" <gi...@apache.org>.
boring-cyborg[bot] commented on PR #75170:
URL: https://github.com/apache/airflow/pull/75170#issuecomment-6279787810
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our [Contributors' Guide](https://github.com/apache/airflow/blob/main/contributing-docs/README.rst)
Here are some useful points:
- Pay attention to the quality of your code (ruff, mypy and type annotations). Our [prek-hooks]( https://github.com/apache/airflow/blob/main/contributing-docs/08_static_code_checks.rst#prerequisites-for-prek-hooks) will help you with that.
- In case of a new feature add useful documentation (in docstrings or in `docs/` directory). Adding a new operator? Check this short [guide](https://github.com/apache/airflow/blob/main/airflow-core/docs/howto/custom-operator.rst) Consider adding an example Dag that shows how users should use it.
- Consider using [Breeze environment](https://github.com/apache/airflow/blob/main/dev/breeze/doc/README.rst) for testing locally, it's a heavy docker but it ships with a working Airflow and a lot of integrations.
- Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
- Please follow [ASF Code of Conduct](https://www.apache.org/foundation/policies/conduct) for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
- Be sure to read the [Airflow Coding style]( https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#coding-style-and-best-practices).
- Always keep your Pull Requests rebased, otherwise your build might fail due to changes not related to your commits.
Apache Airflow is a community-driven project and together we are making it better 🚀.
In case of doubts contact the developers at:
Mailing List: dev@airflow.apache.org
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