Skip to content

Resource init/shutdown not called properly with Closing marker. #699

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
kkjot88 opened this issue Apr 25, 2023 · 0 comments
Open

Resource init/shutdown not called properly with Closing marker. #699

kkjot88 opened this issue Apr 25, 2023 · 0 comments

Comments

@kkjot88
Copy link

kkjot88 commented Apr 25, 2023

Consider this example:

class TestClass:
    @inject
    def test(self, resource=Closing[Provide["resource"]]):
        assert resource == "resource"
        print("test_class")


@inject
def global_test(
    resource=Closing[Provide["resource"]],
    test_class1: TestClass = Provide["test_class"],
    test_class2: TestClass = Provide["test_class"],
):
    assert resource == "resource"
    print("Test start.")
    test_class1.test()
    test_class2.test()
    print("Test end.")


def init_resource():
    print("Init resource. >>")
    yield "resource"
    print("Shutdown resource. <<")


class Container(containers.DeclarativeContainer):
    wiring_config = containers.WiringConfiguration(modules=[__name__])
    resource = providers.Resource(init_resource)
    test_class = providers.Factory(TestClass)


c = Container()

global_test()

Result:

Init resource. >>
Test start.
test_class
Shutdown resource. <<
Init resource. >>
test_class
Shutdown resource. <<
Test end.

To me it seems init_resource() should be called three times, first time when global_test() execution starts (since resource is injected), then second and third time on each of test_class.test() calls. Result shows otherwise.

I did some debugging and this is what i figured out, see comments:

cpdef object _provide(self, tuple args, dict kwargs):
    if self.__initialized:	
        return self.__resource
# global_test() exeuction.
def _get_sync_patched(fn, patched: PatchedCallable):
    @functools.wraps(fn)
    def _patched(*args, **kwargs):
        cdef object result
        cdef dict to_inject
        cdef object arg_key
        cdef Provider provider

        to_inject = kwargs.copy()
        for arg_key, provider in patched.injections.items():
            if arg_key not in kwargs or isinstance(kwargs[arg_key], _Marker):
            	# provider.initialized == False, resource gets initialized.
                to_inject[arg_key] = provider()

	# then we go back to global_test() and into test_class1.test() wrapper right away.
        result = fn(*args, **to_inject)

        if patched.closing:
            for arg_key, provider in patched.closing.items():
                if arg_key in kwargs and not isinstance(kwargs[arg_key], _Marker):
                    continue
                if not isinstance(provider, providers.Resource):
                    continue
                # this does nothing since provider.initialized was set to False during
                # the execution of test_class1.test().
                # provider.initialized == False
                provider.shutdown()

        return result
    return _patched


# test_clast1.test() execution.
def _get_sync_patched(fn, patched: PatchedCallable):
    @functools.wraps(fn)
    def _patched(*args, **kwargs):
        cdef object result
        cdef dict to_inject
        cdef object arg_key
        cdef Provider provider

        to_inject = kwargs.copy()
        for arg_key, provider in patched.injections.items():
            if arg_key not in kwargs or isinstance(kwargs[arg_key], _Marker):
            	# this time around provider is already initialized so it is not initialized again,
            	# value is returned from the if self._initialized statement.
            	# provider.initialized == True, resource doesn't get initialized.
                to_inject[arg_key] = provider()

	# test_class1.test() proceeds as expected.
        result = fn(*args, **to_inject)

        if patched.closing:
            for arg_key, provider in patched.closing.items():
                if arg_key in kwargs and not isinstance(kwargs[arg_key], _Marker):
                    continue
                if not isinstance(provider, providers.Resource):
                    continue
                # resource is shutdown after test_class1.test() execution ends,
                # provider.initliazed == True
                provider.shutdown()
                # provider.initialized == False, was set to False in shutdown

        return result
    return _patched

Not sure what is the proper way to tackle this problem without major reworks while still being clean about it. Maybe whenever resource is being provided with Closing marker it should be copied so there is no conflict in provider.initialized checks. Not sure if this is inline with the purpose of Resource thou nor if that would do the trick and not break something else.

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

No branches or pull requests

1 participant