Skip to content

AIP-84: Migrate Extra Links endpoint to fastapi - #44277

Merged
pierrejeambrun merged 10 commits into
apache:mainfrom
astronomer:extra_link_endpoint
Nov 25, 2024
Merged

AIP-84: Migrate Extra Links endpoint to fastapi#44277
pierrejeambrun merged 10 commits into
apache:mainfrom
astronomer:extra_link_endpoint

Conversation

@prabhusneha

@prabhusnehaprabhusneha commented Nov 22, 2024

Copy link
Copy Markdown
Contributor

Related: #42370

Testing:

API responses using legacy and fastAPI endpoints:

Legacy-API:

image

FastAPI:

image


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

@boring-cyborgboring-cyborgBot added area:API Airflow's REST/HTTP API area:UI Related to UI/UX. For Frontend Developers. labels Nov 22, 2024
@vatsrahul1001vatsrahul1001 added the legacy api Whether legacy API changes should be allowed in PR label Nov 22, 2024
@prabhusneha
prabhusneha marked this pull request as ready for review November 22, 2024 14:28
Comment threadairflow/api_fastapi/core_api/routes/public/extra_links.py Outdated
Comment threadairflow/api_fastapi/core_api/routes/public/extra_links.py
Comment threadairflow/api_fastapi/core_api/routes/public/extra_links.py
Comment threadairflow/api_fastapi/core_api/routes/public/__init__.py
Comment threadairflow/api_fastapi/core_api/routes/public/extra_links.py Outdated
Comment threadtests/api_fastapi/core_api/routes/public/test_extra_links.py Outdated
@prabhusneha
prabhusnehaforce-pushed the extra_link_endpoint branch 2 times, most recently from 11340ed to 8fda581CompareNovember 24, 2024 15:34
Comment threadairflow/api_fastapi/core_api/routes/public/extra_links.py Outdated

@amoghrajeshamoghrajesh 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.

Good work on the PR @prabhusneha!
Mostly looks fine, have a few comments and the static checks needs fixing.

Comment threadairflow/api_fastapi/core_api/datamodels/extra_links.py
Comment threadairflow/api_fastapi/core_api/routes/public/extra_links.py
Comment threadtests/api_fastapi/core_api/routes/public/test_extra_links.py
Comment threadairflow/api_fastapi/core_api/routes/public/extra_links.py Outdated

@pierrejeambrunpierrejeambrun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Great thanks.

Looking good overall, a few suggestions, and should be ready to merge once other comments are resolved.

Comment threadairflow/api_fastapi/core_api/datamodels/extra_links.py
Comment threadairflow/api_fastapi/core_api/routes/public/extra_links.py
Comment threadairflow/api_fastapi/core_api/routes/public/extra_links.py Outdated
@prabhusneha

Copy link
Copy Markdown
ContributorAuthor

Great thanks.

Looking good overall, a few suggestions, and should be ready to merge once other comments are resolved.

Rebased and resolved all comments.

@pierrejeambrun

Copy link
Copy Markdown
Member

Thanks 👍

@pierrejeambrun
pierrejeambrun merged commit 1351b08 into apache:mainNov 25, 2024
@pierrejeambrun
pierrejeambrun deleted the extra_link_endpoint branch November 25, 2024 16:53
got686-yandex pushed a commit to got686-yandex/airflow that referenced this pull request Jan 30, 2025
* migrate extra link endpoint to fastapi
* add airflow license
* add tests
* Apply suggestions from code review
Co-authored-by: Kalyan R <kalyan.ben10@live.com>
* add teardown in test
* change async to sync
* Address PR comments and fix static checks
* Address PR comment
---------
Co-authored-by: Sneha Prabhu <snehaprabhu@Snehas-MacBook-Pro.local>
Co-authored-by: Kalyan R <kalyan.ben10@live.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:APIAirflow's REST/HTTP APIarea:UIRelated to UI/UX. For Frontend Developers.legacy apiWhether legacy API changes should be allowed in PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@prabhusneha@pierrejeambrun@rawwar@amoghrajesh@vatsrahul1001