Skip to content

Resource Factory for Short-Lived Session Objects #455

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

Open
moon-bits opened this issue May 12, 2021 · 10 comments
Open

Resource Factory for Short-Lived Session Objects #455

moon-bits opened this issue May 12, 2021 · 10 comments
Assignees
Labels

Comments

@moon-bits
Copy link

moon-bits commented May 12, 2021

Hi @rmk135

You did a great job with this library! Thank you very much for that!

Yet, I'm having a hard time with a very common approach when using Resource: A database session should be created for each
instance of get_user.

def create_db_session(db: Database):
  session = db.create_session()
    yield session
  db.close_session(session)

class APIContainer:
  database_session = providers.Resource(create_db_session)
  
class UseCaseContainer:
  api = providers.DependenciesContainer()

  get_user = providers.Factory(
    use_cases.GetUser, 
    database_session=api.database_session # `database_session` must be created for every instance of `get_user`
  ) 

But when running the application, it only creates ONE database session for the whole lifetime of the application, not N (the number of API calls).

How is it possible to create a database session for each get_user instance?

@moon-bits
Copy link
Author

@rmk135 do you need more information? I'm currently stucked in my project due this obstacle 😿

@rmk135
Copy link
Member

rmk135 commented May 17, 2021

Hey @moon-bits ,

You need to shutdown resources explicitly. In that case resource will be initialized again and you'll have a new database connection for each get_user() call.

For instance, if you use Flask and you'd like to have a "request scope" singleton, that's what you can do: https://python-dependency-injector.ets-labs.org/providers/singleton.html#implementing-scopes

You can make the same thing with a resource provider. Just call .shutdown() instead of .reset().

PS: Apologies for the delayed response.

@rmk135 rmk135 self-assigned this May 17, 2021
@moon-bits
Copy link
Author

Hi @rmk135 ,

I see. I wanted something more out-of-the-box by not calling .reset() or .shutdown() explicitly.

Do you think my use case can also be achieved by a Factory instead of a Resource? If so, how would you refactor the above mentioned code?

Thanks again for your help and work!

@rmk135
Copy link
Member

rmk135 commented May 17, 2021

I see. I wanted something more out-of-the-box by not calling .reset() or .shutdown() explicitly.

I understand. The problem is that it's unknown when you expect to call db.close_session(session).

Do you think my use case can also be achieved by a Factory instead of a Resource? If so, how would you refactor the above mentioned code?

You can change Resource provider to Factory. The factory should look like this: Factory(db.create_session). This will create a connection for every get_user call, but this doesn't solve connection closing problem by its own.

I would suggest you to try this approach:

  1. Change Resource to Factory as you mentioned.
  2. Pass database provider to the use case
  3. Manage connection lifetime inside of the use case
class Container();

    database = provider.Factory(db.create_db, ...)

    get_user = providers.Factory(
        use_cases.GetUser,
        database_session_provider=api.database_session.provider,
) 

class GetUser:

    def __init__(self, database_session_provider):
        self._database_session_provider = database_session_provider

    def execute(self):
        with self._database_session_provider() as session:  # session is created here
            ...
        # and closed after "with" block is over

@moon-bits
Copy link
Author

Thanks @rmk135 !

Apologies for asking you again, but now I remember why I wanted it to behave like a Resource.

The thing is that get_user might have also some other Factory arguments that must use the same database session (used for transactional purposes).

So the requirement is to have a get_user instance that gets f.e. Repository instances injected but all instances need to use the same database session.

i.e.

class RepositoryContainer:
  user_repository = providers.Factory(
    MyRepository,
    database_session=api.database_session # must be the same database session instance as in `get_user`
  )

class UseCaseContainer:
  api = providers.DependenciesContainer()

  get_user = providers.Factory(
    use_cases.GetUser, 
    database_session=api.database_session # `database_session` must be created for every instance of `get_user`
    user_repository=...
  ) 

The example linked in the documentation is nice, but unfortunately not practical as the database session is only used within one repository.

@rmk135
Copy link
Member

rmk135 commented May 18, 2021

Ok, I see what you're looking for. I don't think there is anything out-of-the box to make it work that way. This is an interesting problem. I would like to address it in the framework one day.

@m-vellinga
Copy link

Running into the same situation. Would love to have a database session that is essentially tied to a request/response cycle and all the dependancies that use it during that request/response cycle and closes up afterwards. It first sight the "Closing" marker does what we want but that only seems to work in combination with "Provide" when directly referencing the "Resource" not on dependancies that have the resource as a sub dependancy.

You would also need an idempotent shutdown for that on the Resource whenever you Provide multiple dependancies wrapped in a Closing that all reference that single Resource.

@thatguysimon
Copy link
Contributor

Having the same issue as well.
@m-vellinga @moon-bits did you manage to come up with a decent workaround for this?

@m-vellinga
Copy link

Having the same issue as well. @m-vellinga @moon-bits did you manage to come up with a decent workaround for this?

Sadly no, I was using FastAPI as HTTP framework so opted to switch (back) to the builtin DI solution that FastAPI provides.

@kiriharu
Copy link

kiriharu commented Oct 25, 2022

Same situation, I'm trying to resolve this with "Closing" marker, but it's not working:

# handlers
@inject
async def create_project(
    request: web.Request,
    project_repository: ProjectRepository = Closing[Provide[Container.project_repository]],
) -> web.Response:
    ....
# di
class Container(containers.DeclarativeContainer):
    db_session = providers.Resource(get_session, database=db)
    project_repository = providers.Factory(
        ProjectRepository,
        session=db_session
    )

But if I add Closing[Provide[Container.db_session]] to handler args - all works correctly.

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

No branches or pull requests

5 participants