You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "namita-can (via GitHub)" <gi...@apache.org> on 2026/07/23 20:54:29 UTC
[PR] Fix airflowctl trigger sending unrecognized fields to older API servers [airflow]
namita-can opened a new pull request, #70328:
URL: https://github.com/apache/airflow/pull/70328
DagOperations.trigger() didn't exclude None fields when serializing the request, unlike create_event() in the same file. This meant every trigger request included partition_key and bundle_version (added in Airflow 3.2.0), which older API servers reject as unrecognized fields.
Adds exclude_none=True to match create_event()'s existing pattern, plus a regression test asserting unset fields are excluded from the request body.
Manually verified against a live Airflow 3.1.x deployment (Charmed Airflow on Kubernetes): confirmed dags trigger fails with the extra_forbidden error on the unpatched code, and succeeds cleanly — both with and without --logical-date passed explicitly, once the fix is applied.
* closes: #70327
<!-- 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: https://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.
For user-facing UI changes, please attach before/after screenshots (or a short
screen recording) so reviewers can assess the visual impact.
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:
* 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`, in [airflow-core/newsfragments](https://github.com/apache/airflow/tree/main/airflow-core/newsfragments). You can add this file in a follow-up commit after the PR is created so you know the PR number.
--
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 airflowctl trigger sending unrecognized fields to older API servers [airflow]
Posted by "boring-cyborg[bot] (via GitHub)" <gi...@apache.org>.
boring-cyborg[bot] commented on PR #70328:
URL: https://github.com/apache/airflow/pull/70328#issuecomment-5063336105
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
Re: [PR] Fix airflowctl trigger sending unrecognized fields to older API servers [airflow]
Posted by "namita-can (via GitHub)" <gi...@apache.org>.
namita-can commented on code in PR #70328:
URL: https://github.com/apache/airflow/pull/70328#discussion_r3779965286
##########
airflow-ctl/tests/airflow_ctl/api/test_operations.py:
##########
@@ -1157,6 +1157,36 @@ def handle_request(request: httpx.Request) -> httpx.Response:
response = client.dags.trigger(dag_id=self.dag_id, trigger_dag_run=self.trigger_dag_run)
assert response == self.dag_run_response
+ def test_trigger_excludes_none_fields_from_request_body(self):
+ """Regression test: trigger() must exclude unset (None) fields from the
+ request body. Older API servers reject unrecognized fields (e.g.
+ partition_key, bundle_version, added in a later API version) with a
+ 422 extra_forbidden error if they're present at all, even as null.
+ """
+ captured_body = {}
+
+ def handle_request(request: httpx.Request) -> httpx.Response:
+ nonlocal captured_body
+ captured_body = json.loads(request.content)
+ return httpx.Response(200, json=json.loads(self.dag_run_response.model_dump_json()))
+
+ client = make_api_client(transport=httpx.MockTransport(handle_request))
+ client.dags.trigger(dag_id=self.dag_id, trigger_dag_run=self.trigger_dag_run)
+
+ # conf is explicitly defaulted to {} by trigger() when unset, so it's expected.
+ assert captured_body == {"conf": {}}
+ for field in (
Review Comment:
Reworked the test to assert partition_key/bundle_version are excluded and logical_date is present.
--
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 airflowctl trigger sending unrecognized fields to older API servers [airflow]
Posted by "shahar1 (via GitHub)" <gi...@apache.org>.
shahar1 commented on PR #70328:
URL: https://github.com/apache/airflow/pull/70328#issuecomment-6436679182
CLosing as already covered by #73763
--
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 airflowctl trigger sending unrecognized fields to older API servers [airflow]
Posted by "namita-can (via GitHub)" <gi...@apache.org>.
namita-can commented on code in PR #70328:
URL: https://github.com/apache/airflow/pull/70328#discussion_r3779946266
##########
airflow-ctl/src/airflowctl/api/operations.py:
##########
@@ -603,7 +603,7 @@ def trigger(
trigger_dag_run.conf = {}
try:
self.response = self.client.post(
- f"dags/{dag_id}/dagRuns", json=trigger_dag_run.model_dump(mode="json")
+ f"dags/{dag_id}/dagRuns", json=trigger_dag_run.model_dump(mode="json", exclude_none=True)
Review Comment:
Good catch, @potiuk. Pushed a fix that switches from exclude_none=True to a targeted exclude={"partition_key", "bundle_version"} - this keeps logical_date in the body even when None (since the server requires it, just allows null), while still dropping the two fields older servers reject.
--
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 airflowctl trigger sending unrecognized fields to older API servers [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #70328:
URL: https://github.com/apache/airflow/pull/70328#discussion_r3887415518
##########
airflow-ctl/src/airflowctl/api/operations.py:
##########
@@ -503,8 +503,13 @@ def trigger(
"""Create a Dag run."""
if trigger_dag_run.conf is None:
trigger_dag_run.conf = {}
+ # partition_key and bundle_version were added in Airflow 3.2.0; older API
+ # servers (extra="forbid") reject them as unrecognized fields even when null.
+ # logical_date must stay in the body even when None - the server declares it
+ # without a default, making it a required (though nullable) field.
self.response = self.client.post(
- f"dags/{dag_id}/dagRuns", json=trigger_dag_run.model_dump(mode="json")
+ f"dags/{dag_id}/dagRuns",
+ json=trigger_dag_run.model_dump(mode="json", exclude={"partition_key", "bundle_version"}),
Review Comment:
Excluding these unconditionally means they can never be sent, including to servers that support them — and both are user-settable today (`cli_config.py:714` generates a CLI flag for every model field bar `schema_`), so `--partition-key foo` is silently discarded rather than rejected.
Conditioning the exclusion on the value keeps both sides working:
```suggestion
exclude = {
field
for field in ("partition_key", "bundle_version")
if getattr(trigger_dag_run, field) is None
}
self.response = self.client.post(
f"dags/{dag_id}/dagRuns",
json=trigger_dag_run.model_dump(mode="json", exclude=exclude),
)
```
An old server no longer sees the unknown null fields; a new server still receives values the user actually set. A user who sets `--partition-key` against an old server then gets a 422, which is the correct outcome — that server genuinely cannot honour the request, and saying so is better than pretending it worked.
---
Drafted-by: Claude Code (Opus 5); reviewed by @potiuk 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 airflowctl trigger sending unrecognized fields to older API servers [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #70328:
URL: https://github.com/apache/airflow/pull/70328#discussion_r3682287539
##########
airflow-ctl/tests/airflow_ctl/api/test_operations.py:
##########
@@ -1157,6 +1157,36 @@ def handle_request(request: httpx.Request) -> httpx.Response:
response = client.dags.trigger(dag_id=self.dag_id, trigger_dag_run=self.trigger_dag_run)
assert response == self.dag_run_response
+ def test_trigger_excludes_none_fields_from_request_body(self):
+ """Regression test: trigger() must exclude unset (None) fields from the
+ request body. Older API servers reject unrecognized fields (e.g.
+ partition_key, bundle_version, added in a later API version) with a
+ 422 extra_forbidden error if they're present at all, even as null.
+ """
+ captured_body = {}
+
+ def handle_request(request: httpx.Request) -> httpx.Response:
+ nonlocal captured_body
+ captured_body = json.loads(request.content)
+ return httpx.Response(200, json=json.loads(self.dag_run_response.model_dump_json()))
+
+ client = make_api_client(transport=httpx.MockTransport(handle_request))
+ client.dags.trigger(dag_id=self.dag_id, trigger_dag_run=self.trigger_dag_run)
+
+ # conf is explicitly defaulted to {} by trigger() when unset, so it's expected.
+ assert captured_body == {"conf": {}}
+ for field in (
Review Comment:
This asserts `"logical_date" not in captured_body`, which codifies the behaviour I think is the bug — so the test would keep passing after a fix that reintroduces the field.
More broadly, `httpx.MockTransport` returns a canned 200 without validating the body against the server model, so no test here can catch "the server would have rejected this". A test that round-trips the body through `airflow.api_fastapi.core_api.datamodels.dag_run.TriggerDAGRunPostBody.model_validate(captured_body)` would have caught the missing required field immediately, and is worth adding whatever direction the fix takes.
---
Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting
##########
airflow-ctl/src/airflowctl/api/operations.py:
##########
@@ -603,7 +603,7 @@ def trigger(
trigger_dag_run.conf = {}
try:
self.response = self.client.post(
- f"dags/{dag_id}/dagRuns", json=trigger_dag_run.model_dump(mode="json")
+ f"dags/{dag_id}/dagRuns", json=trigger_dag_run.model_dump(mode="json", exclude_none=True)
Review Comment:
`exclude_none=True` drops **every** null field, including `logical_date`, which the server requires to be present (see the body of my review). It also can't distinguish "never provided" from "explicitly set to null", so a user who deliberately passes a null value has it silently dropped.
The stated problem is narrower than this: older servers reject *fields they don't know about*. So the targeted fix is to omit only those, e.g. `exclude={"partition_key", "bundle_version"}` when talking to an older server, ideally gated on a negotiated API version rather than applied unconditionally.
If you'd rather keep it general, `exclude_unset=True` is at least semantically honest about "the caller didn't set this" — but it has the same `logical_date` problem, so it doesn't avoid the regression on its own.
---
Drafted-by: Claude Code (Opus 5); reviewed by @potiuk 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 airflowctl trigger sending unrecognized fields to older API servers [airflow]
Posted by "shahar1 (via GitHub)" <gi...@apache.org>.
shahar1 closed pull request #70328: Fix airflowctl trigger sending unrecognized fields to older API servers
URL: https://github.com/apache/airflow/pull/70328
--
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