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

BackgroundTasks don't run #212

Closed
jfeaver opened this issue Sep 19, 2024 · 6 comments · Fixed by #213
Closed

BackgroundTasks don't run #212

jfeaver opened this issue Sep 19, 2024 · 6 comments · Fixed by #213

Comments

@jfeaver
Copy link

jfeaver commented Sep 19, 2024

Describe the bug
BackgroundTasks don't run.

To Reproduce
Use this Cadwyn app:

from cadwyn import Cadwyn, VersionBundle, HeadVersion, Version, VersionedAPIRouter
from fastapi import BackgroundTasks


def write_notification(email: str, message=""):
    with open("log.txt", mode="w") as email_file:
        content = f"notification for {email}: {message}"
        email_file.write(content)


router = VersionedAPIRouter()


@router.post("/send-notification/{email}")
async def send_notification(email: str, background_tasks: BackgroundTasks):
    background_tasks.add_task(write_notification, email, message="some notification")
    return {"message": "Notification sent in the background"}


app = Cadwyn(versions=VersionBundle(HeadVersion(), Version("2000-01-01")))
app.generate_and_include_versioned_routers(router)
  1. Send a POST to the /send-notification/{email} route.
  2. See that the expected message is returned ("Notification sent in the background").
  3. Look for a log file from the background task. It isn't there.

Expected behavior
A log file should be created with the provided email param logged.

This vanilla FastAPI App has the expected behaviour:

from fastapi import BackgroundTasks, FastAPI

app = FastAPI()

def write_notification(email: str, message=""):
    with open("log.txt", mode="w") as email_file:
        content = f"notification for {email}: {message}"
        email_file.write(content)

@app.post("/send-notification/{email}")
async def send_notification(email: str, background_tasks: BackgroundTasks):
    background_tasks.add_task(write_notification, email, message="some notification")
    return {"message": "Notification sent in the background"}

Operating system
MacOS 14.3.1 (23D60)

Additional context
cadwyn v4.2.3
fastapi v0.115.0

> poetry env info
Virtualenv
Python:         3.12.3
Implementation: CPython
Path:           /Users/nathan/Library/Caches/pypoetry/virtualenvs/example_background_task_bug--VJlo0R1-py3.12
Executable:     /Users/nathan/Library/Caches/pypoetry/virtualenvs/example_background_task_bug--VJlo0R1-py3.12/bin/python
Valid:          True

Base
Platform:   darwin
OS:         posix
Python:     3.12.3
Path:       /opt/homebrew/opt/python@3.12/Frameworks/Python.framework/Versions/3.12
Executable: /opt/homebrew/opt/python@3.12/Frameworks/Python.framework/Versions/3.12/bin/python3.12
@zmievsa
Copy link
Owner

zmievsa commented Sep 19, 2024

Thanks! I'll try to provide the fixes tomorrow!

@zmievsa
Copy link
Owner

zmievsa commented Sep 20, 2024

Found the reason. Will publish the fix within 1-3 days.

@jfeaver
Copy link
Author

jfeaver commented Sep 20, 2024

Thanks for the updates!

@zmievsa zmievsa linked a pull request Sep 21, 2024 that will close this issue
@zmievsa
Copy link
Owner

zmievsa commented Sep 21, 2024

@jfeaver please, check version 4.2.4 and see if it fixes your problem. I have also added a unit test for this behavior to make sure we never have such issues again.

@jfeaver
Copy link
Author

jfeaver commented Sep 23, 2024

Great work. Thanks! Background tasks are working as expected for me.

@zmievsa
Copy link
Owner

zmievsa commented Sep 23, 2024

@jfeaver thank you for publishing these bug reports! They really do help a lot.

If you have any extra feedback, any questions, or requests for Cadwyn -- feel free to ping me here or in our discord.

If you need any help integrating Cadwyn into your organization or consulting regarding API design/versioning -- feel free to contact me as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants