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.

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

AIP-84: Migrate Dag Parsing endpoint to FastApi #44416

Merged
merged 8 commits into from
Nov 28, 2024

Conversation

prabhusneha
Copy link
Contributor

@prabhusneha prabhusneha commented Nov 27, 2024

Related: #42370

Testing:

API responses using legacy and fastAPI endpoints:

Legacy-API:

image
last_parsed_time changed in airflow.dag:

image

FastAPI:

image
last_parsed_time changed in airflow.dag:

image

@boring-cyborg boring-cyborg bot added area:API Airflow's REST/HTTP API area:UI Related to UI/UX. For Frontend Developers. labels Nov 27, 2024
@prabhusneha prabhusneha force-pushed the migrate_dag_parsing_endpoint branch from bad0ea3 to af1827d Compare November 27, 2024 12:38
@rawwar rawwar added the legacy api Whether legacy API changes should be allowed in PR label Nov 27, 2024
@rawwar rawwar closed this Nov 27, 2024
@rawwar rawwar reopened this Nov 27, 2024
@rawwar
Copy link
Collaborator

rawwar commented Nov 27, 2024

closed and reopened to run tests

Copy link
Member

@pierrejeambrun pierrejeambrun left a comment

Choose a reason for hiding this comment

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

Nice.

Looking good overall, just a few suggestions :)

@prabhusneha prabhusneha force-pushed the migrate_dag_parsing_endpoint branch from 941c83d to 094966a Compare November 28, 2024 07:55
@pierrejeambrun pierrejeambrun added legacy api Whether legacy API changes should be allowed in PR and removed legacy api Whether legacy API changes should be allowed in PR labels Nov 28, 2024
@prabhusneha
Copy link
Contributor Author

Fixing the checks.

@prabhusneha prabhusneha force-pushed the migrate_dag_parsing_endpoint branch from c32838e to b3b783d Compare November 28, 2024 11:52
@prabhusneha
Copy link
Contributor Author

Rebased and all PR comments addressed.. FYI @pierrejeambrun

Copy link
Member

@pierrejeambrun pierrejeambrun left a comment

Choose a reason for hiding this comment

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

One small nit, ready to merge

airflow/api_fastapi/core_api/routes/public/dag_parsing.py Outdated Show resolved Hide resolved
@prabhusneha prabhusneha force-pushed the migrate_dag_parsing_endpoint branch from 5fb527b to 5546e23 Compare November 28, 2024 15:44
@prabhusneha prabhusneha force-pushed the migrate_dag_parsing_endpoint branch from 5546e23 to 2cae524 Compare November 28, 2024 16:14
@pierrejeambrun
Copy link
Member

Thanks!

@pierrejeambrun pierrejeambrun merged commit 3e427c9 into apache:main Nov 28, 2024
66 checks passed
@pierrejeambrun pierrejeambrun deleted the migrate_dag_parsing_endpoint branch November 28, 2024 16:41
LefterisXefteris pushed a commit to LefterisXefteris/airflow that referenced this pull request Jan 5, 2025
* AIP-84: Migrate Dag Parsing endpoint to FastApi

* Address PR comments

* Change the test class name

* Address PR comments and fix tests

* Address PR comment

* remove database isolation option to fix failing check

* Address PR comment

---------

Co-authored-by: Sneha Prabhu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:API Airflow's REST/HTTP API area:UI Related to UI/UX. For Frontend Developers. legacy api Whether legacy API changes should be allowed in PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants