Skip to content

calling a common function that uses a db session from inside fastapi as well as celery #68

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

Closed
sandys opened this issue Sep 22, 2019 · 10 comments

Comments

@sandys
Copy link

sandys commented Sep 22, 2019

hi guys,
I have a pretty common usecase that im not able to figure out how to do - i have a common function that i need to call using fastapi as well as a celery worker.

However, im not able to figure out how to create the session in a way that it will work for both api and worker.

the code that is in this repo does not have an example which i could find, but it is super common in real world.

Could you guys comment on how we should write this ? If you could just add a function that calls a db query and use that function in both the api and celery worker, that should be enough.

we have a pretty hacky way of doing this in flask - check if a flask request context exists, etc etc . I'm hoping there's a much more cleaner way here.

@ghost
Copy link

ghost commented Oct 3, 2019

Hi sandys,

You can import Session from app.db.session in your worker.py file and use it inside your tasks.

Cheers

@sandys
Copy link
Author

sandys commented Oct 3, 2019 via email

@dmontagu
Copy link
Contributor

dmontagu commented Oct 3, 2019

Just put the session creation function in a self contained module that doesn't import fastapi code.

Then import that function into your fastapi code, and into your celery code.

@sandys
Copy link
Author

sandys commented Oct 3, 2019 via email

@dmontagu
Copy link
Contributor

dmontagu commented Oct 3, 2019

we are hoping this means that fastapi will create a new database session
for every fastapi-session/request .
I hope this is correct ?

Yes that's right.

I see two options.


Option 1: Move the config to a self-contained file that doesn't have fastapi as a dependency. Then the session.py module could be imported safely. This might not be desirable if the only setting you need in the workers is the sqlalchemy uri though.


Option 2: Make a get_sessionmaker_instance function that takes a URI as an argument:

from sqlalchemy import create_engine
from sqlalchemy.orm import scoped_session, sessionmaker

# from app.core import config  # no longer necessary

def get_sessionmaker_instance(uri: str) -> sessionmaker:
    engine = create_engine(uri, pool_pre_ping=True)
    db_session = scoped_session(
        sessionmaker(autocommit=False, autoflush=False, bind=engine)
    )
    Session = sessionmaker(autocommit=False, autoflush=False, bind=engine)
    return Session

Then, instead of importing Session from this module, you import the function, and optionally create a Session in an app-specific file via
Session = get_sessionmaker_instance(config.SQLALCHEMY_DATABASE_URI), and do the same in worker code. Note -- to avoid reducing to option 1, you'll need a way to get the SQLALCHEMY_DATABASE_URI that doesn't depend on the config file. But that's going to be unavoidable if you are explicitly trying to not share dependencies that you want to use while generating your config.

@sandys
Copy link
Author

sandys commented Oct 3, 2019 via email

@dmontagu
Copy link
Contributor

dmontagu commented Oct 3, 2019

@sandys What is the starlette vs pydantic config bug? I'm curious

@sandys
Copy link
Author

sandys commented Oct 3, 2019 via email

@tiangolo
Copy link
Member

Thanks for all the help here @dmontagu ! 🙇 🍰

@sandys I see you posted a link to another post asking about configs, here are the new FastAPI docs for configs and settings: https://fastapi.tiangolo.com/advanced/settings/

Thanks for reporting back and closing the issue 👍

@sandys
Copy link
Author

sandys commented Apr 14, 2020

closed with thanks

@sandys sandys closed this as completed Apr 14, 2020
alejsdev added a commit that referenced this issue Dec 19, 2024
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

No branches or pull requests

3 participants