Skip to content

Add deadline alert coverage to Dag endpoint tests - #72296

Merged
shahar1 merged 1 commit into
apache:mainfrom
ColtenOuO:add-deadline-alert-dag-endpoint-tests-50992
Oct 4, 2026
Merged

shahar1 merged 1 commit into
apache:mainfrom
ColtenOuO:add-deadline-alert-dag-endpoint-tests-50992

Conversation

@ColtenOuO

@ColtenOuO ColtenOuO commented Aug 30, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Although PR #64815 was closed, issue #50992 remains on the roadmap for AIP-86, so I've opened this PR to address it.

  • Add tests verifying the public DAG API endpoints work correctly with deadline-bearing DAGs
  • Cover GET /dags, GET /dags/{id}, GET /dags/{id}/details, PATCH /dags/{id}, PATCH /dags, and DELETE /dags/{id}
  • Add clear_db_deadline and clear_db_deadline_alert to test cleanup to prevent FK constraint issues

Details

The public DAG endpoint tests (test_dags.py) never created DAGs with deadline alerts, so there was
no coverage for how the API behaves when DagModel._deadline has data and
SerializedDagModel.deadline_alerts has FK-linked records.

7 new test methods across 6 test classes. No existing tests modified.

closes: #50992


Was generative AI tooling used to co-author this PR?
  • Yes — Claude Code (Sonnet 5)

@ferruzzi

ferruzzi commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Does shivaam deserve co-author credit or is this a substantial change over the closed PR?

@ferruzzi ferruzzi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple nits, otherwise LGTM

Comment thread airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_dags.py Outdated
@ColtenOuO
ColtenOuO force-pushed the add-deadline-alert-dag-endpoint-tests-50992 branch from 96b9877 to e3b86e6 Compare September 9, 2026 16:52
@ColtenOuO

Copy link
Copy Markdown
Contributor Author

Does shivaam deserve co-author credit or is this a substantial change over the closed PR?

Sure! I think this is worthful to add shivaam to co-author, because the overall logic was based on his implementation.

@ferruzzi

ferruzzi commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

LGTM. Thanks for adding the tests

@ferruzzi

ferruzzi commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

@bbovenzi - Did you ant to review this since it's touching the API? It's just adding additional testing, so shouldn't be a problem.

@shahar1 shahar1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please fix conflicts?

@shahar1 shahar1 changed the title Add deadline alert coverage to DAG endpoint tests Add deadline alert coverage to Dag endpoint tests Sep 18, 2026
@ColtenOuO
ColtenOuO force-pushed the add-deadline-alert-dag-endpoint-tests-50992 branch from dfb0c46 to 43fb073 Compare September 19, 2026 01:31
@ColtenOuO

ColtenOuO commented Sep 19, 2026 •

Copy link
Copy Markdown
Contributor Author

Could you please fix conflicts?

Sure! fixed it.

The public Dag endpoint tests never exercised Dags with deadline
alerts, leaving no coverage for how the API behaves when
DagModel._deadline has data and SerializedDagModel.deadline_alerts
has FK-linked records.

closes: apache#50992

Co-Authored-By: Shivam Rastogi <6463385+shivaam@users.noreply.github.com>
@ColtenOuO
ColtenOuO force-pushed the add-deadline-alert-dag-endpoint-tests-50992 branch from 6735cf0 to 5f2c56f Compare September 28, 2026 15:09
@shahar1
shahar1 merged commit 7fad35d into apache:main Oct 4, 2026
78 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update dag endpoint tests to include dags with deadlines

4 participants