[24.1] Don't use async def where not appropriate

[mvdbeek]

Anywhere we interact with the database or the file system or we make outgoing calls using functions that are not awaitable we shouldn't use async def. So basically very, very few API routes that just return in-memory data can use async def.

If in doubt, use a normal sync definition. async dependencies with sync routes are ok (see 7b54d64 as an example for how to make it work if you do have something that needs to be awaited but you're also making blocking calls), FastAPI will run the async dependencies in the main async loop and dispatch any sync dependency to a threadpool. However async routes will run straight on the event loop, and I/O will block the event loop. If the event loop is blocked all other requests will stall, and on top of that the gunicorn liveness probe will likely time out, resulting in https://sentry.galaxyproject.org/share/issue/a04cd4cea5fb45dc9594fc7d1215b1c0/, which will eventually kill the worker and all requests currently in flight.

Of course other things can make the request arbiter decide to stop the worker, but what I've done here should be correct. All of these routes either query the database (directly or through lazy loading attributes) or touch files on the file system.

How to test the changes?

(Select all options that apply)

• I've included appropriate automated tests.
• This is a refactoring of components with existing test coverage.
• Instructions for manual testing are as follows:
  1. [add testing steps and prerequisites here if you didn't write automated tests covering all your changes]

License

• I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

[bgruening]

Just to prove my understanding, is the following correct?

• If we are using a non-async library for database or file system access (as we do today), we should avoid using async def because you're not gaining any non-blocking benefits.
• However, if you're using an async-compatible library (e.g., asyncpg for databases or aiofiles for file operations), async def can be beneficial.
• Accessing in-memory data usually don’t perform heavy I/O operations, async def might not be necessary in this case as well.

[mvdbeek]

• If we are using a non-async library for database or file system access (as we do today), we should avoid using async def because you're not gaining any non-blocking benefits.

Yes, but blocking the event loop isn't only "not gaining", it's actively degrading everything else being handled on the same event loop.

• However, if you're using an async-compatible library (e.g., asyncpg for databases or aiofiles for file operations), async def can be beneficial.

Yes, it is more efficient to not dispatch to the threadpool, in theory higher concurrency can be supported that way

• Accessing in-memory data usually don’t perform heavy I/O operations, async def might not be necessary in this case as well.

Right, you don't gain much but you do avoid passing through the thread pool.