@dependency_definition doesn't work for generator functions with a Container parameter

[rowanbarrie]

One more issue to throw your way @meadsteve 😅

It seems @dependency_definition doesn't work for generator functions with the c: Container parameter, e.g.:

@dependency_definition(container)
def _session_dependency(container: Container) -> Generator[Session, None, None]:
    read_only: bool = container[Request].scope["db_access"] == "READ_ONLY"
    with get_db_session(read_only) as session:
        yield session

This gives the following error:

ImportError while loading conftest '/home/rbarrie/code/gdb/tests/unit/api/conftest.py'.
tests/unit/api/conftest.py:10: in <module>
    from gdb.entrypoints.bootstrap import deps
gdb/entrypoints/bootstrap.py:133: in <module>
    def _session_dependency(container: Container) -> Session:
../../.cache/pypoetry/virtualenvs/gdb-wpT7sdDb-py3.8/lib/python3.8/site-packages/lagom/decorators.py:69: in _decorator
    definition_func, return_type = _extract_definition_func_and_type(func)  # type: ignore
../../.cache/pypoetry/virtualenvs/gdb-wpT7sdDb-py3.8/lib/python3.8/site-packages/lagom/decorators.py:102: in _extract_definition_func_and_type
    return value_from_gen, return_type.__args__[0]  # todo: something less hacky
E   AttributeError: type object 'Session' has no attribute '__args__'

For comparison a simple generator function without the c: Container parameter works fine:

@dependency_definition(container)
def _session_dependency() -> Generator[Session, None, None]:
    with get_db_session(False) as session:
        yield session

I'm not sure if this is by design or not?

I may be being too ambitious here but I want to have each of my FastAPI endpoints determine (via a custom APIRoute + Request.scope attribute) which DB transaction / connection is used (read/write vs read-only). I suppose I could achieve this with two independent Lagom containers, but then I would have two of every dependency initialised...

[meadsteve]

This is an interesting issue. On the surface this is just a bug. Lagom has support for generators as a dependency_definition and it looks like there's a bug with generators taking an argument as this was never tested. This should be fairly straightforward to fix.

The possibly more subtle bug is when would you expect this ContextManager to exit? The lagom container itself has no concept of request lifetimes or anything like that so it currently would perform the __exit__ straight after loading the session but I wonder in this case would you expect it to happen at the end of the request? If you would this is something that would need to be built into the lagom-fastapi integration.

Something like the following might work as a manual solution to this problem:

@dependency_definition(container)
def _session_dependency(container: Container) -> ContextManagert[Session]:
    read_only: bool = container[Request].scope["db_access"] == "READ_ONLY"
    return get_db_session(read_only)


# Later in the request

def some_request(session_manager = deps. depends(ContextManagert[Session])):
    with session_manager as session:
         # do something with the session
         pass 

[rowanbarrie]

The lagom container itself has no concept of request lifetimes or anything like that so it currently would perform the exit straight after loading the session but I wonder in this case would you expect it to happen at the end of the request?

Hmm that is indeed what I (perhaps naively) was expecting...

Unfortunately I want the DB Session instance injected into a repository-like dependency, where a with statement wouldn't make sense. I.e. Not at the FastAPI route level but further down the dependency tree, like this:

class FooRepository:
    def __init__(self, session: Session):
        self.session = session

    def get_foo():
        return self.session.query(Foo).one()

I take it the same problem (__exit__ being called immediately after DI is completed) would occur even in a non-FastAPI context? In which case I may have to re-think my approach....I believe the FastAPI built-in DI system supports generator lifetimes, although I'm not so sure now 🤔

[meadsteve]

If the fastapi built in DI system supports generators being valid for the lifetime of a request then it should be possible to hook into it from lagom. I'll need to do a little reading.

[meadsteve]

I've split this more complicated problem off as #180

[meadsteve]

So the inconsistency between generators taking an argument and not has been fixed by 502043b. I'll deal with the rest in #180