Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

Depends object returned instead of session #1693

Closed
bryan-bar opened this issue Jul 8, 2020 · 20 comments
Closed

Depends object returned instead of session #1693

bryan-bar opened this issue Jul 8, 2020 · 20 comments

Comments

@bryan-bar
Copy link

bryan-bar commented Jul 8, 2020

I am currently trying to migrate from flask to fastapi and trying to keep my files separated. My depends is not called from my routes but from within my crud file but similar to the documentation for relational databases: https://fastapi.tiangolo.com/tutorial/sql-databases/

This was my initial way of using a database session which is similar to the sqlalchemy docs
Also, is this bad practice or is it okay to use this and a mix of depends? I did kind of want to decouple sqlalchemy from fastapi.

def get_database():
    db = scoped_session(default_session)
    try:
        yield db
    finally:
        db.close()

def get_references(db: scoped_session = next(get_database())):
    references = db.query(Reference).all()
    return references

I then tried to switch and use Depends but am getting the same issue as others below with my attempts added at the end:

I am seeing this same issue in .52 using almost verbatim the doc examples with postgres. Depends is not calling the function but returning what appears to be an instantiate Depends object.

@pspierce and @H0r53, Did you solve this? I am seeing in 0.58.1. I also tried: instead of using generator, returning the db directly -- same problem.

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

engine = create_engine('postgresql://postgres:postgres@localhost:5432/postgres', echo=True)
default_session = scoped_session(sessionmaker(autocommit=False,
                                         autoflush=False,
                                         bind=engine))

def get_database():
    db = default_session()
    try:
        yield db
    finally:
        db.close()

First error I got:

AttributeError: 'Depends' object has no attribute 'query'

from database.data import get_database, scoped_session
from database.models.reference import Reference
from fastapi import Depends


def get_references(db: scoped_session = Depends(get_database())):
    references = db.query(Reference).all()
    return references

Without Depends:

AttributeError: 'generator' object has no attribute 'query'

def get_references(db: scoped_session = get_database()):

Originally posted by @BrainABar in #631 (comment)

@SirTelemak
Copy link

SirTelemak commented Jul 13, 2020

As I can see, you calling get_database directly in function arguments, but you have to pass function object into Depends in order to make injection. In your case you should rewrite def get_references as:

def get_references(db: scoped_session = Depends(get_database)):
    references = db.query(Reference).all()
    return references

@Kludex
Copy link
Sponsor Collaborator

Kludex commented Jul 13, 2020

Just to complement: Depends receives a callable.
There's something that there's a miss concept when you do this:

def get_references(db: scoped_session = Depends(get_database())):

Notice that Depends is not receiving a callable, you're calling get_database(). Which means that Depends will not receive the callable, and will try to solve it without success. That's why you're having the current error.

As @SirTelemak said, you should use this notation:

def get_references(db: scoped_session = Depends(get_database)):

And here you can see that get_database is not being called. Now Depends can successfully solve it.

Ps.: You can still use the last notation, without Depends, but the problem is that you're getting a generator in your snippet. To solve it you should use next. I don't recommend this approach, but I'll show you anyway:

def get_references(db: scoped_session = next(get_database())):

or:

def get_references(db: scoped_session = get_database()):
    # some_var = next(db)

@bryan-bar
Copy link
Author

Ps.: You can still use the last notation, without Depends, but the problem is that you're getting a generator in your snippet. To solve it you should use next. I don't recommend this approach, but I'll show you anyway:

def get_references(db: scoped_session = next(get_database())):

or:

def get_references(db: scoped_session = get_database()):
    # some_var = next(db)

Why do you not recommend this approach? My first example was like the example you put.

I ended getting the initial approach cause I was declaring in its own module. I ended up reading somewhere that it might be because it was outside of my routes but I did not dig further yet since I was unsure if sqlalchemy's way was enough for my use case.

@SirTelemak
Copy link

Why do you not recommend this approach? My first example was like the example you put.

First of all, that will be executed only once when this function imported, so you losing your "one session for one call" functionality and if something happens during handling db request you can lose session and won't be able to reopen it again.
Second, tear-down won't be working since it's feature of Depends. So you should manually call next(db) but it will close connection and won't be reopened again as I said (if you using second variant), in first variant you unable to close connection at all

So, with this approach you lose versatility and control in your program,

@GabrielCarpr
Copy link

Third reason not to use that approach is that you lose the dependency injection/substitution feature of FastAPI, which makes it very easy to swap out dependencies when running tests, for example to use a testing database.

@bryan-bar
Copy link
Author

@SirTelemak
I was misunderstanding how Depends worked cause I was trying to make it work around my code and outside of a request context. I tried to keep the database like its own package and wanted my functions to handle themselves. The database connections would be tied to a function call so that the endpoint would just call references.get() and I wouldn't have to worry about how it works again or what a db connection even was.

I read more into the docs and the replies and I understand the benefits much more now. I was confused so I had to step away and read through everything again to see why my way wasn't working how I wanted.

I was able to find a way to make the session recreate if I wanted to do it using reload so my functions would be reimported and I would get a new connection.

@GabrielCarpr, however, brought up testing. It be harder with my approach cause then pytest would need to know how to configure my database for testing so I would lose any benefits I thought I would be achieving.

Thank you all for the help.

@H0r53
Copy link

H0r53 commented Jul 29, 2020

I'm still receiving AttributeError: 'Depends' object has no attribute 'query' even when I re-write my api routes to use the db_session=Depends(get_db) syntax.

Note that using db_session=next(get_db()) does work, at least for a while - I'm noticing strange behavior after running the API for over 30 minutes such as DB connection failures or random errors related to transactions being rolled back during a flush. I can only assume these errors are related to this underlying setup issue.

My get_db code is simply defined as such:

engine = create_engine(api_settings.database_uri, pool_pre_ping=True, pool_size=30, max_overflow=30, echo_pool=True, echo=True, pool_recycle=3600)  # recycle every hour
Session = scoped_session(sessionmaker(autocommit=False, autoflush=True, bind=engine))

# Method to get db instance
def get_db():
    db = Session()
    try:
        yield db
    finally:
        db.close()

I've sent so many flags to create_engine because I'm finding that FastAPI/SQLAlchemy crashes when I'm making multiple requests in a fast timeframe.

Why is this issue still occurring and how can it be resolved?

@GabrielCarpr
Copy link

GabrielCarpr commented Jul 29, 2020

@H0r53 can you include a full example, including your route?

@H0r53
Copy link

H0r53 commented Jul 29, 2020

Here is an example that issues simple token authentication derived from the FastAPI examples. The notable code for get_db was provided in my previous comment. The error is: AttributeError: 'Depends' object has no attribute 'query' - This happens in the crud_user.user.get_by_username that expects a valid db session, not a Depends object.

Edited to show relevant code

def get_user_by_username(username: str):
    return crud_user.user.get_by_username(db_session=Depends(get_db), username=username)

def authenticate_user(username: str, password: str):
    user = get_user_by_username(username)
    if not user:
        return False
    if not verify_password(password, user.hashed_password):
        return False
    return user


####################
# Routes
####################

@router.post("/token", response_model=Token)
def login_for_access_token(form_data: OAuth2PasswordRequestForm = Depends()):
    user = authenticate_user(form_data.username, form_data.password)
    if not user:
        raise HTTPException(
            status_code=HTTP_401_UNAUTHORIZED,
            detail="Incorrect username or password",
            headers={"WWW-Authenticate": "Bearer"},
        )
    access_token_expires = timedelta(minutes=api_settings.access_token_expire_minutes)
    access_token = create_access_token(
        data={"sub": user.username}, expires_delta=access_token_expires
    )
    return {"access_token": access_token, "token_type": "bearer"}

@GabrielCarpr
Copy link

@H0r53 you can't use Depends in your own functions, it has to be in FastAPI functions, mainly routes. You can, however, use Depends in your own functions when that function is also a dependency, so could can have a chain of functions.

Eg, a route uses Depends to resolve a 'get_current_user', which also uses Depends to resolve 'get_db', and the whole chain will be resolved. But if you then call 'get_current_user' without using Depends, it won't be able to resolve 'get_db'.

What I do is get the DB session from the route and then pass it down through every layer and function. I believe this is also better design.

@H0r53
Copy link

H0r53 commented Jul 29, 2020

So just for clarity, only use Depends on routes?

@GabrielCarpr
Copy link

Yes, unless you're using it in a function that itself is resolved by Depends. But for simplicity, keep it in the route

@H0r53
Copy link

H0r53 commented Jul 29, 2020

This has been a nightmare over the past several months involving several roundabout "solutions." After following your advice everything appears to be working and I'm hoping this resolves the other strange session related issues that I've been experiencing. Thank you!

@GabrielCarpr
Copy link

@H0r53 I went through the same, including next(get_db). Perhaps I should make a PR for this as the docs are obviously missing something

@tiangolo
Copy link
Owner

tiangolo commented Dec 9, 2020

Thanks for the help here everyone! 👏 🙇

Thanks for reporting back and closing the issue 👍

@CommonCrisis
Copy link

Hi,

how should I handle then getting a session outside FastAPI route functions?
I now use next(get_database) - but I'm pretty sure that screws up my SQL Connection pool after some time cause the application dies.

Like on startUp I get some data from a SQL DB - putting the functionality in a route makes no sense to me.
Or for my SessionManager I need to fetch some data from SQL DB.

Thanks :)

@Kludex
Copy link
Sponsor Collaborator

Kludex commented Jan 11, 2021

@CommonCrisis My recommendation is to use the context manager way: https://fastapi.tiangolo.com/tutorial/dependencies/dependencies-with-yield/#using-context-managers-in-dependencies-with-yield

@svkoskin
Copy link

svkoskin commented Oct 6, 2022

@H0r53 you can't use Depends in your own functions, it has to be in FastAPI functions, mainly routes. You can, however, use Depends in your own functions when that function is also a dependency, so could can have a chain of functions.

Eg, a route uses Depends to resolve a 'get_current_user', which also uses Depends to resolve 'get_db', and the whole chain will be resolved. But if you then call 'get_current_user' without using Depends, it won't be able to resolve 'get_db'.

This is a very good comment which should be somehow included in https://fastapi.tiangolo.com/tutorial/dependencies/.

I'm pretty angry that the official page fails to mention this very concrete limitation - even though it makes perfect sense from the code execution point of view. The documentation tries to conceal how the code is executed - while adding the insult of stressing the simplicity and effortlessness, making the document readers having failed to use the feature feel themselves complete morons. The expectation of magic fails, since DI does not get invoked in most functions in a typical program (which is natural when thought out with realistic expectations regarding magic in programming).

I'd expect to see when the DI machinery is actually triggered, even though it means loosing the illusion of unicorns and magic. I have so far every time failed to use use the DI in one of our applications, without having any support from the documentation.

@JarroVGIT
Copy link
Contributor

Feel free to provide a comment to the documentation with a PR. I would disagree with most of your statements and nowhere in the docs is implied that the DI system of FastAPI works outside the context of FastAPI.

If you think its insulting that the easiness of said methodology is stressed in the docs, then you might be able to rephrase those bits as well if your at it :)

@svkoskin
Copy link

svkoskin commented Oct 7, 2022

Feel free to provide a comment to the documentation with a PR. I would disagree with most of your statements and nowhere in the docs is implied that the DI system of FastAPI works outside the context of FastAPI.

If you think its insulting that the easiness of said methodology is stressed in the docs, then you might be able to rephrase those bits as well if your at it :)

I'm sorry, but I can't agree I could write any acceptable documentation about a feature that I still don't understand after several months of fighting against. I think it would be wasting anyone's time.

I'm personally very much opposed to "intuitive and powerful" implicit machinery, they don't fit my mental model. They set expectations which don't hold for first time users if something changes or goes wrong. That is my personal limitation which can't be generalised without additional evidence.

Repository owner locked and limited conversation to collaborators Feb 28, 2023
@tiangolo tiangolo converted this issue into discussion #7334 Feb 28, 2023

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Projects
None yet
Development

No branches or pull requests

9 participants