Add name fields to SDK deadline alerts#64926
Conversation
There was a problem hiding this comment.
Pull request overview
Propagates DeadlineAlert.name through SDK/core serialization into the metadata DB and adjusts the UI API/OpenAPI shapes for deadline-related responses.
Changes:
- Add optional
nameto the Task SDKDeadlineAlertand include it in core serialization encode/decode. - Persist
nameintoDeadlineAlertDB rows when creating alerts from serialized DAG data, and updatenamewhen reusing existing deadline UUIDs. - Remove
description/alert_descriptionfrom the UI API models and the generated UI OpenAPI client types; addDeadlinesto the UIMenuItemenum.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| task-sdk/src/airflow/sdk/definitions/deadline.py | Adds optional name to SDK DeadlineAlert. |
| airflow-core/src/airflow/serialization/encoders.py | Emits name in serialized deadline alert payloads. |
| airflow-core/src/airflow/serialization/decoders.py | Parses name from serialized deadline alert payloads. |
| airflow-core/src/airflow/serialization/definitions/deadline.py | Adds field constant + name to SerializedDeadlineAlert. |
| airflow-core/src/airflow/models/serialized_dag.py | Writes/updates DeadlineAlertModel.name when persisting serialized DAGs. |
| airflow-core/src/airflow/api_fastapi/core_api/datamodels/ui/deadline.py | Removes description fields from UI API response models. |
| airflow-core/src/airflow/api_fastapi/common/types.py | Adds DEADLINES to MenuItem. |
| airflow-core/src/airflow/api_fastapi/core_api/openapi/_private_ui.yaml | Removes description fields from schemas; adds Deadlines to menu enum. |
| airflow-core/src/airflow/ui/openapi-gen/requests/types.gen.ts | Regenerated client types (drops description fields; adds Deadlines). |
| airflow-core/src/airflow/ui/openapi-gen/requests/schemas.gen.ts | Regenerated client schemas (drops description fields; adds Deadlines). |
| airflow-core/tests/unit/models/test_deadline_alert.py | Removes assertions/data for description. |
| airflow-core/tests/unit/api_fastapi/core_api/routes/ui/test_deadlines.py | Removes alert_description assertions and related test naming/docs. |
Comments suppressed due to low confidence (1)
airflow-core/src/airflow/serialization/encoders.py:218
encode_deadline_alert()uses a literal "name" key while the decoder and other call sites useDeadlineAlertFields.NAME. For consistency and to avoid drift/typos, consider usingDeadlineAlertFields.NAMEhere as well.
from airflow.sdk.serde import serialize
return {
"name": getattr(d, "name", None),
"reference": encode_deadline_reference(d.reference),
"interval": d.interval.total_seconds(),
"callback": serialize(d.callback),
}
There was a problem hiding this comment.
Overall looks good to me, just one nit.
I'll leave the final word to @ferruzzi on the name update logic.
|
Why are we removing description? I think other than that it looks fine from my side |
We are removing description because we decided that we should not rely on description in telling the users what the deadline and deadline alerts are. |
|
I'm not sure I follow. If the user wants to set a description, why not let them? Let's say we work for Big Company. The person in Marketing who is using the deadline may not be the dev/admin person who wrote the code for the deadline. Having a description seems reasonable to me. |
Having interval + reference and name should provide enough detail about the deadline without the ui feeling bloated. Because I did add description before and it felt like there was too much info and a bit confusing. |
amoghrajesh
left a comment
There was a problem hiding this comment.
Need clarification on why we are dropping this as it looks to be a breaking change. If this is intentional, we will need a newsfragment
Changes that I flagged didn't make it in a release
|
@ferruzzi @amoghrajesh I told Richard that we could ignore descriptions in favor of a good name + a human readable interval+reference string. But since 3.2.1 is about to be released and there could still be a reason for a description then I am ok with putting description back. My apologies for adding extra confusion |
|
Alight, let's give it a try. Worst case, we can always add a Description field back later if users ask for it, right? |
There was a problem hiding this comment.
Approving with a nit that you can take or leave.
I'd also want to point out that this does not remove "description" from the ORM so the database still has that row.. we just never use it.
Approved on my end with the requirement that one of the UI folks also approves from their end, just to be safe.
Co-authored-by: D. Ferruzzi <ferruzzi@amazon.com>
pierrejeambrun
left a comment
There was a problem hiding this comment.
From API / UI point of view, looking good
Backport failed to create: v3-2-test. View the failure log Run detailsNote: As of Merging PRs targeted for Airflow 3.X In matter of doubt please ask in #release-management Slack channel.
You can attempt to backport this manually by running: cherry_picker e9d1066 v3-2-testThis should apply the commit to the v3-2-test branch and leave the commit in conflict state marking After you have resolved the conflicts, you can continue the backport process by running: cherry_picker --continueIf you don't have cherry-picker installed, see the installation guide. |
|
Nice, congrats |
|
Manual backport #65601 |
* Add deadlines support with name and description fields in alerts and UI * Add 'viewAll' label to deadlineStatus in dag.json * Refactor deadlineAlerts referenceType structure * Refine deadline alert translations in dag.json Updated deadline alert messages for clarity and consistency. * Add completion rule text for deadline alerts in UI * Remove duplicate completionRule entry in deadlineAlerts * Remove alert description from DeadlineAlert and related models * Enhance deadline handling: return name updates alongside UUID mapping in SerializedDagModel * Add alert_id field to DeadlineResponse and update tests for alert handling * Remove DEADLINES option from MenuItem enum * Update airflow-core/src/airflow/serialization/encoders.py --------- (cherry picked from commit e9d1066) Co-authored-by: Richard Wu <richard9@ualberta.ca> Co-authored-by: D. Ferruzzi <ferruzzi@amazon.com>
Adds name and alert id fields to DeadlineAlert, exposed through the API. (json edited files are for ui change for my pr in the future) @bbovenzi
Was generative AI tooling used to co-author this PR?
{pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.