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

Make list backfills endpoint use asyncio #44208

Merged
merged 1 commit into from
Nov 22, 2024

Conversation

dstandish
Copy link
Contributor

@dstandish dstandish commented Nov 20, 2024

This is a sort of hello world / proof of concept for having an route implemented using asyncio.

Depends on #44226

@dstandish dstandish changed the title Make list backfills endpoint use async Make list backfills endpoint use asyncio Nov 20, 2024
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.

Looks nice.

Just a few small suggestions.

Also I realized that we cannot have 'lazy' loading of some attributes at serialization time (when using pydantic), because this will be 'uncontroled async I/O'.

I don't know if you are encountering this on the Backfill model, but basically it means that everything needs to be eager loaded before, so pydantic does not emit uncontrolled request afterwards. (and unfortunatly we cannot await BackfillResponse.model_validate)

Also just to mention that there is a stream API to iterate over multiple results. That could be interesting to see if this can help us here.

airflow/api_fastapi/common/db/common.py Outdated Show resolved Hide resolved
airflow/api_fastapi/common/db/common.py Outdated Show resolved Hide resolved
@dstandish dstandish force-pushed the poc-async-route-fastapi branch from 74cbd14 to 1efe639 Compare November 20, 2024 21:01
airflow/settings.py Outdated Show resolved Hide resolved
@dstandish dstandish force-pushed the poc-async-route-fastapi branch 3 times, most recently from a9c6980 to f835db5 Compare November 21, 2024 22:34
@dstandish dstandish marked this pull request as ready for review November 21, 2024 22:42
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. Feel free to merge after resolving conflicts.

@pierrejeambrun
Copy link
Member

pierrejeambrun commented Nov 22, 2024

Note to myself to not merge too early:

Depends on #44267

@dstandish dstandish force-pushed the poc-async-route-fastapi branch from 2939aa3 to 73b1174 Compare November 22, 2024 19:27
@dstandish dstandish merged commit 66407e8 into apache:main Nov 22, 2024
94 of 95 checks passed
@dstandish dstandish deleted the poc-async-route-fastapi branch November 22, 2024 20:37
LefterisXefteris pushed a commit to LefterisXefteris/airflow that referenced this pull request Jan 5, 2025
This is a sort of hello world / proof of concept for having an route implemented using asyncio.  Gotta start somewhere.
got686-yandex pushed a commit to got686-yandex/airflow that referenced this pull request Jan 30, 2025
This is a sort of hello world / proof of concept for having an route implemented using asyncio.  Gotta start somewhere.
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.

2 participants