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

Allow provider to be a context manager (sync/async) #92

Merged
merged 3 commits into from
Nov 23, 2023

Conversation

fzyukio
Copy link
Contributor

@fzyukio fzyukio commented Nov 14, 2023

Allow a provider to be used as context manager

Problem statement:

  • A variable provided through context manager is useful in case it is a resource that needs to be cleaned up immediately after the caller has finished. Example of this is a PGBouncer session.
  • Bind a class to an instance of a context manager (through bind or bind_to_constructor) or to a function decorated as a context manager leads to the context manager to be used as is, not via with statement

Proposed change:

  • In async_injection_wrapper and injection_wrapper: extract the provided instances from kwargs if they are subclasses of AbstractContextManager or AbstractAsyncContextManager
  • Stack them up in a list (or two lists in case of async - we allow async func to be a mixture of sync and async ContextManagers )
  • Before executing the function, use ExitStack (and/or AsyncExitStack) to enter all contexts, the return values are updated to kwargs and finally sent to the function

Testing

  • One new unittest added

@ivankorobkov
Copy link
Owner

Hi!

Quite an interesting change 👍. Never thought about such a case. Thank you.
Could you please add comments to your code, explaining what it does? As you did in the pull request description.

Then I can merge PR and release the next version.

@fzyukio
Copy link
Contributor Author

fzyukio commented Nov 21, 2023

Hi @ivankorobkov thanks for checking it out. I've restructured it a bit and added some comments.

@ivankorobkov
Copy link
Owner

Hi, thank you.

I'll merge it tomorrow and make the next release.

@ivankorobkov ivankorobkov merged commit a1c5b7b into ivankorobkov:master Nov 23, 2023
@ivankorobkov
Copy link
Owner

Done https://pypi.org/project/inject/5.2.0/

@fzyukio
Copy link
Contributor Author

fzyukio commented Nov 23, 2023

@ivankorobkov Awesome! Thanks a lot for your quick action 👍

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 this pull request may close these issues.

2 participants