You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "1fanwang (via GitHub)" <gi...@apache.org> on 2026/05/12 15:30:31 UTC

[PR] Validate DagRun conf payload size at trigger boundary [airflow]

1fanwang opened a new pull request, #66787:
URL: https://github.com/apache/airflow/pull/66787

   Triggering a Dag with an oversized `conf` payload currently produces a generic 500. The DagRun row is created in memory, the size error surfaces only at flush time deep in SQLAlchemy as `(1406, "Data too long for column 'conf' at row 1")` on MySQL, and the caller has no signal that conf size was the cause. Bug #14159 from 2021 covered the same crash class against the deprecated experimental API; the same failure mode is reproducible against the FastAPI public API today.
   
   This adds a JSON-size check at the trigger boundary so the request is rejected before the row reaches the DB, with a message that points at the right fix (XCom / Variables / external storage).
   
   ## Why
   
   Reproducer (against any 3.0+ deployment on MySQL with default `innodb_default_row_format`):
   
   ```
   curl -X POST $API/api/v2/dags/example_bash_operator/dagRuns \
     -H "Content-Type: application/json" \
     -d "{\"conf\":{\"k\":\"$(python -c 'print("x"*70000)')\"}}"
   ```
   
   Response: 500. Server log:
   
   ```
   sqlalchemy.exc.DataError: (pymysql.err.DataError)
   (1406, "Data too long for column 'conf' at row 1")
   [SQL: INSERT INTO dag_run (...) VALUES (...)]
   ```
   
   ## What
   
   - New `[core] max_dagrun_conf_size_bytes` (default 65535) bounds the JSON-encoded conf size. `0` disables the check.
   - New `airflow.exceptions.DagRunConfTooLargeError` with `status_code = 413` carries the measured size and limit.
   - `SerializedDAG.create_dagrun()` validates before insert via the new `validate_dagrun_conf_size()` helper, so the CLI and `TriggerDagRunOperator` paths get the same check as the REST API.
   - The FastAPI `POST /dags/{dag_id}/dagRuns` handler maps the exception to `413 Payload Too Large` with the actionable message.
   
   The default of 65535 fits the smallest MySQL `JSON` column variant; Postgres and larger MySQL row formats can raise it.
   
   ## Tests
   
   `airflow-core/tests/unit/models/test_dagrun.py::TestValidateDagRunConfSize` covers the helper (None / empty / at-limit / over-limit / disabled / multibyte UTF-8). `test_dag_run.py::TestTriggerDagRun::test_dagrun_creation_conf_too_large_returns_413` covers the route-level mapping to 413.
   
   ## Risk
   
   Backwards-incompatible only for deployments that today rely on MySQL silently rejecting oversized conf (those see a generic 500). The check is bounded by a config and can be disabled.
   
   Closes #66779
   


-- 
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] Validate DagRun conf payload size at trigger boundary [airflow]

Posted by "choo121600 (via GitHub)" <gi...@apache.org>.
choo121600 commented on PR #66787:
URL: https://github.com/apache/airflow/pull/66787#issuecomment-4440507075

   @1fanwang Converting to **draft** — this PR doesn't yet meet our [Pull Request quality criteria](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-quality-criteria).
   
   - :x: **Pre-commit / static checks** — Failing: `CI image checks / Static checks`. See [docs](https://github.com/apache/airflow/blob/main/contributing-docs/08_static_code_checks.rst).
   - :x: **Provider tests** — Failing: `provider distributions tests / Compat 3.0.6:P3.10:`, `provider distributions tests / Providers sdist tests`. See [docs](https://github.com/apache/airflow/blob/main/contributing-docs/12_provider_distributions.rst).
   
   See the linked criteria for how to fix each item, then mark the PR "Ready for review". This is **not** a rejection — just an invitation to bring the PR up to standard. No rush.
   
   ---
   
   _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] Validate DagRun conf payload size at trigger boundary [airflow]

Posted by "1fanwang (via GitHub)" <gi...@apache.org>.
1fanwang commented on PR #66787:
URL: https://github.com/apache/airflow/pull/66787#issuecomment-4445493556

   Closing in favour of #66888.
   
   After more thought, the original proposal here adds a config knob, a new exception class, and per-route validation for what is fundamentally a "DB rejected the payload" failure that the FastAPI exception-handler layer can translate for free across every endpoint. The new PR ships exactly one handler for `sqlalchemy.exc.DataError`, registers it on both the public REST API and the execution API, and inherits the translation on every existing and future write endpoint (DagRun `conf`, Connection `extra`, Variable `val`, XCom `value`, TaskInstance `note`, HITL fields, etc) with no new configuration surface. Leaving this PR open as reference for the trade-off conversation.
   
   ---
   Drafted-by: Claude Code (Opus 4.7) (no human review 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] Validate DagRun conf payload size at trigger boundary [airflow]

Posted by "1fanwang (via GitHub)" <gi...@apache.org>.
1fanwang closed pull request #66787: Validate DagRun conf payload size at trigger boundary
URL: https://github.com/apache/airflow/pull/66787


-- 
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