You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "sjyangkevin (via GitHub)" <gi...@apache.org> on 2026/06/08 03:24:58 UTC
[PR] Fix: allow deadline alert UUID references in serialized Dag schema [airflow]
sjyangkevin opened a new pull request, #68195:
URL: https://github.com/apache/airflow/pull/68195
<!-- 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
-->
* related: #58248
* related: #67449
When a Dag defines a deadline alert, the alert is stored in its own `deadline_alert` table on write, and `SerializedDagModel._generate_deadline_uuids` rewrites the serialized Dag's `deadline` field from a list of encoded dicts into a list of UUID strings pointing at those rows. However, `schema.json` only allowed `deadline` to be a dict, an array of dicts, or null. As a result, validating the stored serialized Dag for any deadline-bearing Dag fails with something like:
```
FAILED airflow-core/tests/unit/models/test_serialized_dag.py::TestSerializedDagModel::test_write_dag[compress-serialized_dags] - jsonschema.exceptions.ValidationError: ['019ea3de-70d1-77ec-82b1-0dceeb508be5'] is not valid under any of the given schemas
Failed validating 'anyOf' in schema['allOf'][0]['properties']['dag']['properties']['deadline']:
{'anyOf': [{'$ref': '#/definitions/dict'},
{'items': {'$ref': '#/definitions/dict'}, 'type': 'array'},
{'type': 'null'}]}
On instance['dag']['deadline']:
['019ea3de-70d1-77ec-82b1-0dceeb508be5']
```
**Fix**
- Adds an `array` of `string` option to the `deadline` schema so it accepts the persisted UUID-reference format.
- Adds a regression test that writes a deadline-bearing Dag and asserts the stored data passes schema validation, guarding against the schema and storage format drifting apart again.
---
##### 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".
-->
- [X] Yes (please specify the tool below)
Generated-by: [Claude Code (Opus 4.8)] 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: allow deadline alert UUID references in serialized Dag schema [airflow]
Posted by "ferruzzi (via GitHub)" <gi...@apache.org>.
ferruzzi commented on PR #68195:
URL: https://github.com/apache/airflow/pull/68195#issuecomment-4663615343
This doesn't feel right. Where in the code are you actually running into this failure? `validate_schema` is (currently??) only ever called before the Deadline dict is converted to a UUID.
It looks like this is happening because `test_write_dag` is using `LazyDeserializedDAG` twice, once before the dict>UUID migration then again after and that is causing a mismatch. If that's the case then this isn't actually fixing the bug, it's just hiding it. I think a better fix would be to actually fix write_dag so that `_generate_deadline_uuids` is working on a deepcopy of dag.data and not pulling the rug out from under `LazyDeserializedDAG` by changing things in place. Something like
```python
working_data = copy.deepcopy(dag.data)
deadline_uuid_mapping = cls._generate_deadline_uuids(working_data)
```
--
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: allow deadline alert UUID references in serialized Dag schema [airflow]
Posted by "ferruzzi (via GitHub)" <gi...@apache.org>.
ferruzzi closed pull request #68195: Fix: allow deadline alert UUID references in serialized Dag schema
URL: https://github.com/apache/airflow/pull/68195
--
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: allow deadline alert UUID references in serialized Dag schema [airflow]
Posted by "ferruzzi (via GitHub)" <gi...@apache.org>.
ferruzzi commented on PR #68195:
URL: https://github.com/apache/airflow/pull/68195#issuecomment-5321030766
I'm about to merge https://github.com/apache/airflow/pull/70148 which covers the same issue.
--
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: allow deadline alert UUID references in serialized Dag schema [airflow]
Posted by "sjyangkevin (via GitHub)" <gi...@apache.org>.
sjyangkevin commented on PR #68195:
URL: https://github.com/apache/airflow/pull/68195#issuecomment-4663663209
Thanks for the feedback and the direction. I will dive in to understand that behaviour, make sure i understand the actual logic, to properly implement 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] Fix: allow deadline alert UUID references in serialized Dag schema [airflow]
Posted by "ferruzzi (via GitHub)" <gi...@apache.org>.
ferruzzi commented on PR #68195:
URL: https://github.com/apache/airflow/pull/68195#issuecomment-4813233117
I think I agree with what you said but I'm having a bit of a brain-fog day, so here is what I think, just in case:
1. keep schema change and test you added here so the validation can be run at any time and not just pre-update
2. add the deepcopy() to fix write_dag
--
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: allow deadline alert UUID references in serialized Dag schema [airflow]
Posted by "sjyangkevin (via GitHub)" <gi...@apache.org>.
sjyangkevin commented on PR #68195:
URL: https://github.com/apache/airflow/pull/68195#issuecomment-4775238311
Hi @ferruzzi , I did some analysis and trying to understand better the root cause for the test failure and also to understand better the implementation. The current implementation in the PR updates the `schema.json` to let it accept a list of strings, to make it "pass" the check. In short, the root cause is that the way deadline is persisted in serialized_dag getting updated but the test seems to validate on the previous format :thinking: . I think I agree with you that this change does hide these details in the process, and I am leaning towards to the solution you proposed to update `write_dag` and make it to work on a deepcopy of the data instead of modifying in place. Let me know if you think it is still a good direction to go with. Thanks!
The blob in `serialized_dag.data` goes through two transformations.
In the below code snippet, `deadline` is serialized into `dict` and `serialized_dag["deadline"]` is a list of dicts.
https://github.com/apache/airflow/blob/ab4c0978a76e4f44148fccd3a4f4fa28404ca317/airflow-core/src/airflow/serialization/serialized_objects.py#L1704-L1706
The `SerializedDagModel.write_dag` method serialize a DAG and writes it into database. `_generate_deadline_uuids` is called. The dicts are lifted out into the `deadline_alert` table, and the blob is mutated in place to hold an array of UUID strings that reference those rows.
https://github.com/apache/airflow/blob/ab4c0978a76e4f44148fccd3a4f4fa28404ca317/airflow-core/src/airflow/models/serialized_dag.py#L677-L682
When the `test_write_dag` in `airflow-core/tests/unit/models/test_serialized_dag.py::TestSerializedDagModel` run, it validates the persisted data, which is a list of UUID strings, with the shape of data defined in the `schema.json`.
```
result = session.scalar(select(SDM).where(SDM.dag_id == dag.dag_id))
DagSerialization.validate_schema(result.data)
```
However, the schema (schema.json:196-205) only allows three shapes:
https://github.com/apache/airflow/blob/918124cb4171ecf739122365db4dee00a3d7a9f8/airflow-core/src/airflow/serialization/schema.json#L196-L205
The list of UUID strings doesn't match with any of the shapes, so raised the validation error.
I also attempted to analyze and understand the implementation, below are some findings, but feel free to correct me if I am wrong.
#58248
#61702
1. It promotes deadline alerts into its own tables, to better support UI for query, sort, etc.
2. At runtime the scheduler must point from a materialized deadline back to the alert definition that produced it. Each alert gets a UUID primary key. The `deadline` table carries a `deadline_alert_id` foreign key to it, and the Dag blob keeps the same UUID as its reference, so both the runtime row and the Dag point at one shared identity.
3. Re-parsing an unchanged Dag should not produce a new serialized version. Replacing definitions with UUIDs lets `write_dag` control what enters the hash. When a definition is unchanged it reuses the existing UUID, and for a name-only change it updates the table row while keeping the UUID, so the hash stays constant and no new version is created.
--
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