Question: does BackgroundTasks documentation need to be improved?

[teners]

Recently, I've been playing with BackgroundTasks through DI in a FastAPI app.

The app itself has a simple GET request endpoint that proxies requests to a third-party API. It also uses aioredis for caching, thus in case of a cache miss it makes a real request and puts a response into Redis. I've decided to make response caching in the background, so my code looked like this:

async def get_page_revisions(page_id: int, bg_tasks: BackgroundTasks):
    page = await get_cached_revisions(page_id)
    if not page:
        page = await _get_page_revisions(page_id)
        bg_tasks.add(ctx.redis.set, page_id, page)
    return page

This works perfectly fine functionality-wise, but sometimes it spits these logs:

/...venvpath/lib/python3.7/site-packages/uvicorn/main.py:307: RuntimeWarning: coroutine 'wait_ok' was never awaited
  loop.run_until_complete(self.serve(sockets=sockets))
RuntimeWarning: Enable tracemalloc to get the object allocation traceback

I was looking for the wait_ok in the libraries I'm using and finally came across it in aioredis. Except the mentioned before aioredis.command.Redis.set (which is always added into the BackgroundTasks in my code) I'm using only aioredis.command.Redis.get method, and I've made sure every single call is being awaited.

Next, I've started looking through the BackgroundTasks code and found this line:

starlette/starlette/background.py

Line 14 in 06d0fc4

self.is_async = asyncio.iscoroutinefunction(func)

I've managed to figure out the issue: aioredis.command.Redis.set itself is not an async function, but it seems to rather return one through a wild chain of function calls. I didn't bother to untangle it, not sure if it's even necessary.

Finally, I've changed my code a little bit:

bg_tasks.add(asyncio.coroutine(ctx.redis.set), page_id, page)

And it seemed to do the trick (I hope 👀 ).

Now, the thing is the current Starlette's documentation doesn't tell anything about the callable BackgroundTask wraps. Maybe, it should be updated telling a user they can wrap both sync and async functions and warn against these kind of cases such as mine?