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

#139 Auto context for Context Resources #141

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

alexanderlazarev0
Copy link
Collaborator

Implements #139.

@alexanderlazarev0
Copy link
Collaborator Author

Point of discussion:

Currently, this feature is implemented in such a way that a new context is created whenever we attempt to resolve a resource. However, we do not prevent the initialization of the resources' context. For example:

r = providers.ContextResource(some_func, auto_context=True)
with r.sync_context(): # actually makes a new context which will never be used.
    ...

Possible temporary solution:

In the above example, the user is making a logic mistake, however, imagine a scenario where you have a container with many providers:

class Container(BaseContainer):
    # many more providers here
    r = providers.ContextResource(some_func, auto_context=True)

with Container.sync_context(): # actually makes a new context for `r` which will never be used.
    ...

Here, the user also will initialize a context for r for no reason.

So what can be done is: when one enters Container.sync_context(), one can check if the provider is a ContextResource and has auto_context==True and only then initialize a new context.

Possible long-term solution:

It can be argued that we want to know if a user is making an explicit context initialization as in the first example, or if this is done by an internal that-depends call (as in example 2). Then we would raise an error for the user, and for internal calls ignore the instruction. This would require rewriting the current interface to have both public and private calls.

@alexanderlazarev0 alexanderlazarev0 self-assigned this Jan 12, 2025
@lesnik512 lesnik512 linked an issue Jan 13, 2025 that may be closed by this pull request
@alexanderlazarev0 alexanderlazarev0 marked this pull request as draft January 13, 2025 10:52
@alexanderlazarev0
Copy link
Collaborator Author

@lesnik512 Yes my bad I need to rework this.

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.

ContextResource that supports auto-context
1 participant