dmontagu / fastapi-utils

Reusable utilities for FastAPI
MIT License
1.91k stars 166 forks source link

[BUG] @repeat_every blocks app startup on version 0.6.0 #305

Closed jonathanhar closed 3 months ago

jonathanhar commented 4 months ago

Describe the bug On version 0.2.1 there is no problem, but when using 0.6.0 the app doesn't start. Attaching simple code to reproduce

To Reproduce

import logging
from contextlib import asynccontextmanager

import uvicorn
from fastapi import FastAPI
from fastapi_utils.tasks import repeat_every

logging.basicConfig(level=logging.INFO, force=True)
logger = logging.getLogger(__name__)

@asynccontextmanager
async def lifespan(app: FastAPI):
    await do_something()
    yield

app = FastAPI(lifespan=lifespan)

@repeat_every(seconds=1, logger=logger)  # 30 Minutes
async def do_something() -> None:
    logger.info("Removing idle devcontainers...")
    return

@app.get("/")
async def health():
    return 'Success'

if __name__ == "__main__":
    uvicorn.run(app, host="localhost", port=8080)

Expected behavior App should start

Environment:

dwdSF commented 4 months ago

++, the app does not start. It also seems that the option wait_first does not work in 0.6.0

Vanderhoof commented 4 months ago

Mommy always told me: lock your dependencies. But I never listened to her. Now I lost several hours on a deadline week figuring out why the server is not starting. This is very unfortunate, but a life lesson nevertheless

IgnacioHeredia commented 4 months ago

Same issue here.

    from fastapi_utils.tasks import repeat_every
  File "/home/iheredia/anaconda3/lib/python3.8/site-packages/fastapi_utils/__init__.py", line 4, in <module>
    from .cbv_base import Api, Resource, set_responses, take_init_parameters
  File "/home/iheredia/anaconda3/lib/python3.8/site-packages/fastapi_utils/cbv_base.py", line 5, in <module>
    from .cbv import INCLUDE_INIT_PARAMS_KEY, RETURN_TYPES_FUNC_KEY, _cbv
  File "/home/iheredia/anaconda3/lib/python3.8/site-packages/fastapi_utils/cbv.py", line 21, in <module>
    from typing_inspect import is_classvar
ModuleNotFoundError: No module named 'typing_inspect'
jonesbusy commented 4 months ago

Same issue. My app doesn't start after update to 0.6.0

ollz272 commented 4 months ago

Same here, needs fixing asap

mgkid3310 commented 4 months ago

Same here, and during my struggle to fix the issue I found that the typing_inspect module's imported within fastapi-restful but it's not listed as a dependency so it has to be installed separately via pip. Would that be worth a separate issue/PR?

juanggcc commented 4 months ago

This issue broke all my apps that relied on fastapi-utils for cron. Really need to start locking my dependencies.

mgkid3310 commented 4 months ago

Seems like await (https://github.com/dmontagu/fastapi-utils/blob/master/fastapi_utils/tasks.py#L77) is the cause of the problem, it should be asyncio.ensure_future or Thread().start()

Keiishu commented 4 months ago

I just tested replacing (https://github.com/dmontagu/fastapi-utils/blob/master/fastapi_utils/tasks.py#L77) with asyncio.ensure_future(loop()) and it works. Every test passes as well for me. (https://github.com/Keiishu/fastapi-utils/tree/fix/tasks-blocking)

ollz272 commented 4 months ago

@Keiishu can you raise a PR?

Keiishu commented 4 months ago

@ollz272 Sure! Here it is: #308

yuval9313 commented 4 months ago

I've seen another issue aimed to solve another problem that caused this issue, I intend to approve any PR made about it but I want the former issue not to come back

EDIT: Add the PR

mgkid3310 commented 4 months ago

@yuval9313 The former issue and PR aimed to remove @app.on_event("startup") decorator for the repeated functions, but the following comments show mixed results. Plus, the @app.on_event("startup") itself was deprecated from FastAPI, and now @asynccontextmanager and async def lifespan(app: FastAPI): is used. (the @repeat decorator is my in-house code but works similar to @repeat_every before the 0.6.0 bug)

@asynccontextmanager
async def lifespan(app: FastAPI):
    log.info(f'Starting {cfg.api_name} v{cfg.api_version} server')
    await scan_results()

    yield

    log.info(f'Shutting down {cfg.api_name} v{cfg.api_version} server')

@utils_common.repeat(seconds=60, delay=30)
async def scan_results():
    report.scan_results()

app = FastAPI(lifespan=lifespan)

So unlike the issue title is suggesting I don't think such bug (repeat working only once) will happen, and the tests from pytest have confirmed this.

yuval9313 commented 3 months ago

Solved in https://github.com/dmontagu/fastapi-utils/pull/310

RamiAwar commented 3 months ago

@yuval9313 I'm on 0.7.0 (latest) and still facing issues:

    from fastapi_utils.tasks import repeat_every
  File ".../.venv/lib/python3.12/site-packages/fastapi_utils/__init__.py", line 4, in <module>
    from .cbv_base import Api, Resource, set_responses, take_init_parameters
  File ".../.venv/lib/python3.12/site-packages/fastapi_utils/cbv_base.py", line 5, in <module>
    from .cbv import INCLUDE_INIT_PARAMS_KEY, RETURN_TYPES_FUNC_KEY, _cbv
  File ".../.venv/lib/python3.12/site-packages/fastapi_utils/cbv.py", line 21, in <module>
    from typing_inspect import is_classvar
ModuleNotFoundError: No module named 'typing_inspect'

I simply installed with poetry, got 0.7.0, but seems something is missing.

@repeat_every(seconds=2)
async def refresh_registered_views() -> None:
    async with SessionCreator.begin() as session:
        await something(session)

@asynccontextmanager
async def lifespan(_: fastapi.FastAPI) -> AsyncGenerator[None, None]:
    await refresh_registered_views()
    yield
RamiAwar commented 3 months ago

Wait why is the installation docs mentioning another package name?

image

This is different on Github vs docs page.

RamiAwar commented 3 months ago

Okay fixed when I installed with [all]. Nevermind. But docs need correction!

poetry add "fastapi-utils[all]"

yuval9313 commented 3 months ago

Sorry about the docs, the CI should've deployed the latest version with the fix, I'll check it soon!

Gerrit-K commented 3 months ago

I can confirm what @RamiAwar commented. It still is an issue on 0.7.0. Though, I think it's not related to the original issue, but rather just to this comment, for which there is a separate issue: #318.

mgkid3310 commented 3 months ago

I can confirm what @RamiAwar commented. It still is an issue on 0.7.0. Though, I think it's not related to the original issue, but rather just to this comment, for which there is a separate issue: #318.

It should be ok when installed via fastapi-utils[all], as [all] includes all optional dependencies including typing_inspect. Adding typing_inspect to standard install might be a good solution though.