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

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

Closed
jonathanhar opened this issue May 13, 2024 · 20 comments
Closed

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

jonathanhar opened this issue May 13, 2024 · 20 comments
Labels
bug Something isn't working

Comments

@jonathanhar
Copy link

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:

  • OS: Tested on Linux and Windows
  • FastAPI Utils version 0.6.0
  • Python version 3.11
@jonathanhar jonathanhar added the bug Something isn't working label May 13, 2024
@dwdSF
Copy link

dwdSF commented May 13, 2024

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

@Vanderhoof
Copy link

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
Copy link

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
Copy link

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

@ollz272
Copy link
Contributor

ollz272 commented May 17, 2024

Same here, needs fixing asap

@mgkid3310
Copy link
Contributor

mgkid3310 commented May 18, 2024

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
Copy link

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

@mgkid3310
Copy link
Contributor

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
Copy link
Contributor

Keiishu commented May 21, 2024

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
Copy link
Contributor

ollz272 commented May 23, 2024

@Keiishu can you raise a PR?

@Keiishu
Copy link
Contributor

Keiishu commented May 23, 2024

@ollz272 Sure! Here it is: #308

@yuval9313
Copy link
Collaborator

yuval9313 commented Jun 3, 2024

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
Copy link
Contributor

@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
Copy link
Collaborator

Solved in #310

@RamiAwar
Copy link

RamiAwar commented Jun 13, 2024

@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
Copy link

Wait why is the installation docs mentioning another package name?

image

This is different on Github vs docs page.

@RamiAwar
Copy link

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

poetry add "fastapi-utils[all]"

@yuval9313
Copy link
Collaborator

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

@Gerrit-K
Copy link

Gerrit-K commented Jul 8, 2024

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
Copy link
Contributor

mgkid3310 commented Jul 8, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.