fastapi-utils icon indicating copy to clipboard operation
fastapi-utils copied to clipboard

[BUG] @repeat_every() does not run without @app.on_event('startup') decorator

Open caseyjohnsonwv opened this issue 2 years ago • 7 comments

Describe the bug The @repeat_every() decorator does not trigger the function it decorates unless the @app.on_event('startup') decorator is also present.

To Reproduce I created this sample main.py to show the issue I've been seeing.

  1. Copy the below and run python3 main.py. The app will create a local file test.txt and write a Unix timestamp to it every second.
  2. Stop the app and comment out @app.on_event('startup')
  3. Run the app again. The file test.txt is no longer being updated. If you delete this file and restart the app, it will never be created.
import time
from fastapi import FastAPI
from fastapi_utils.tasks import repeat_every
import uvicorn

app = FastAPI()

@app.on_event('startup')
@repeat_every(seconds=1)
def do_something():
    with open('test.txt', 'w') as f:
        f.write(f"{time.time():.0f}")

if __name__ == '__main__':
    uvicorn.run('main:app')

Expected behavior The function do_something() should be writing to the file every 1 second, even with @app.on_event('startup') commented out.

Environment:

  • OS: Windows 10
  • FastAPI Utils: 0.2.1
  • FastAPI: 0.75.0
  • Pydantic: 1.9.0
  • Python: 3.10.4

caseyjohnsonwv avatar Apr 05 '22 06:04 caseyjohnsonwv

I can confirm this. Even swapping the two

from (working)

@app.on_event('startup')
@repeat_every(seconds=1)

into (prints once)

@repeat_every(seconds=1)
@app.on_event('startup')

I changed the test into

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

app = FastAPI()

@app.on_event('startup')
@repeat_every(seconds=0.1, max_repetitions=80)
def do_something():
    print(".", end='')

if __name__ == '__main__':
    uvicorn.run(app)

clemens-tolboom avatar Jun 19 '22 10:06 clemens-tolboom

the same here. works only with startup event

vkomodey avatar Aug 11 '22 11:08 vkomodey

I we don't use @app.on_event('startup') how do the app know when to start the task?

mvoitko avatar Sep 21 '22 13:09 mvoitko

My suggestion would be that the framework scans for all decorator @repeat_every at startup and call them, preferable after that all @app.on_event("startup") has been executed. Comparing with Spring framework that has @scheduled that does exactly the same. Where you can decorate multiple functions/tasks that will be called on at startup. @scheduled decorator can also be configured with an initial delay, that is very nice to have.

lennpet avatar Dec 03 '22 07:12 lennpet

I we don't use @app.on_event('startup') how do the app know when to start the task?

The whole point of writing APIs in frameworks like FastAPI is for them to be opinionated. It's also pretty standard for scheduling frameworks to allow specifying a schedule without explicitly specifying a start time, which then by convention equals to "whenever you can start it ASAP".

It's also pretty pointless to require multiple decorators if one of them will be followed by the other in 99,9% (if not 100%) of the cases.

znstahl avatar Feb 24 '23 08:02 znstahl

I we don't use @app.on_event('startup') how do the app know when to start the task?

It's also pretty pointless to require multiple decorators if one of them will be followed by the other in 99,9% (if not 100%) of the cases.

I think a great example of this in practice is Apache Airflow, which requires a datetime object to specify the earliest invocation of a DAG. This datetime can be in the past, meaning "start this as soon as possible." It would make sense for the framework to do one or both of the following:

  • Infer @repeat_every() to mean "start immediately" when not following another decorator.
  • Explicitly require a datetime object inside of @repeat_every() to specify the first invocation.

caseyjohnsonwv avatar Feb 28 '23 00:02 caseyjohnsonwv

'decorators are wrappers around' and in this case they come from two different tools in the order bottom to top.

Maybe fastapi-util could have 'auto decorated' it but that would mean you can never @repeat_every using a different decorator, ie 'after the 20th request'.

FWIW I learned the order (and more) from https://www.youtube.com/watch?v=QH5fw9kxDQA

clemens-tolboom avatar Mar 02 '23 09:03 clemens-tolboom

This is not true, you need to trigger the repeatable event on a event loop and then it will start running on it's own. It's mentioned in the docs.

AdityaSoni19031997 avatar Feb 23 '24 10:02 AdityaSoni19031997