Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Migrate the public endpoint Delete DAG to FastAPI #42914

Merged
merged 2 commits into from
Oct 15, 2024
Merged

Migrate the public endpoint Delete DAG to FastAPI #42914

merged 2 commits into from
Oct 15, 2024

Conversation

omkar-foss
Copy link
Collaborator

closes: #42650
related: #42370

This migrates the Delete DAG API from api_connexion to api_fastapi, with a supporting test.

@boring-cyborg boring-cyborg bot added the area:UI Related to UI/UX. For Frontend Developers. label Oct 10, 2024
@omkar-foss omkar-foss marked this pull request as draft October 10, 2024 19:27
@omkar-foss omkar-foss marked this pull request as ready for review October 10, 2024 20:01
@omkar-foss omkar-foss requested a review from ashb as a code owner October 10, 2024 20:01
@omkar-foss omkar-foss force-pushed the aip-84/dags/delete-dag branch 2 times, most recently from 317a424 to 2ec7b6d Compare October 11, 2024 16:46
@omkar-foss
Copy link
Collaborator Author

Merge conflict resolved, PR is ready for review.

@omkar-foss
Copy link
Collaborator Author

Tests failing after adding migration marker for legacy APIs. Could someone please add the legacy api label to this PR? Thanks.

@pierrejeambrun pierrejeambrun added the legacy api Whether legacy API changes should be allowed in PR label Oct 11, 2024
@pierrejeambrun
Copy link
Member

Label added :)

@omkar-foss
Copy link
Collaborator Author

The mypy tests for mark_fastapi_migration_done had been failing, so have updated it's signature to be more tolerant here. Tests passing now.

pierrejeambrun
pierrejeambrun previously approved these changes Oct 14, 2024
View reviewed changes
airflow/utils/api_migration.py Show resolved Hide resolved
@pierrejeambrun pierrejeambrun dismissed their stale review October 14, 2024 07:58

Approved by mistake.

pierrejeambrun
pierrejeambrun approved these changes Oct 15, 2024
View reviewed changes
Copy link
Member

@pierrejeambrun pierrejeambrun left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks

@pierrejeambrun pierrejeambrun merged commit 6c9c327 into apache:main Oct 15, 2024
51 checks passed
@omkar-foss omkar-foss deleted the aip-84/dags/delete-dag branch October 15, 2024 09:09
R7L208 pushed a commit to R7L208/airflow that referenced this pull request Oct 17, 2024
@omkar-foss
Migrate the public endpoint Delete DAG to FastAPI (apache#42914)
dabc4f2 
* Migrate the public endpoint Delete DAG to FastAPI

* Refactor tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pierrejeambrun pierrejeambrun pierrejeambrun approved these changes

@bbovenzi bbovenzi Awaiting requested review from bbovenzi bbovenzi is a code owner

@ryanahamilton ryanahamilton Awaiting requested review from ryanahamilton ryanahamilton is a code owner

@jscheffl jscheffl Awaiting requested review from jscheffl jscheffl is a code owner

@ephraimbuddy ephraimbuddy Awaiting requested review from ephraimbuddy ephraimbuddy is a code owner

@ashb ashb Awaiting requested review from ashb

Assignees
No one assigned
Labels
area:UI Related to UI/UX. For Frontend Developers. legacy api Whether legacy API changes should be allowed in PR
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AIP-84 Migrate the public endpoint Delete DAG to FastAPI
2 participants
@omkar-foss @pierrejeambrun