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

Rule request: prefer async for FastAPI dependencies #15279

Open
spladug opened this issue Jan 5, 2025 · 2 comments
Open

Rule request: prefer async for FastAPI dependencies #15279

spladug opened this issue Jan 5, 2025 · 2 comments
Labels
needs-decision Awaiting a decision from a maintainer rule Implementing or modifying a lint rule

Comments

@spladug
Copy link

spladug commented Jan 5, 2025

When a synchronous function is used as a dependency factory for FastAPI, the framework runs the function on a threadpool. This is done even if the factory never does IO, which can be quite wasteful. Since it's super tempting to not async-color the function when it's doing something super simple, it seems like it'd be useful to lint for this to avoid potential performance pitfalls.

See https://github.com/zhanymkanov/fastapi-best-practices?tab=readme-ov-file#prefer-async-dependencies for more info.

@MichaReiser
Copy link
Member

Thanks.

I think the challenge with a rule such as this is how Ruff should quantify "a small non-IO operation"

And threads here also come with a price and limitations, that are redundant, if you just make a small non-I/O operation.

I worry that suggest a rule would have a high false positive rate. It might be a better fit for Ruff once we have a "suggestion" severity that only applies to the IDE context.

@MichaReiser MichaReiser added rule Implementing or modifying a lint rule needs-decision Awaiting a decision from a maintainer labels Jan 6, 2025
@spladug
Copy link
Author

spladug commented Jan 6, 2025

Yeah, that's totally fair. Difficult to tell without some way of identifying a function as pure.

I could maybe see it being an opt-in rule as in an asyncio-primary codebase it's definitely the exception to want the synchronous style for IO anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-decision Awaiting a decision from a maintainer rule Implementing or modifying a lint rule
Projects
None yet
Development

No branches or pull requests

2 participants