Skip to content

Conversation

@LeafyLappa
Copy link

The change allows consumers of the library to modify exactly how handler resource methods are called. My case is dependency injection (with lagom):

class ResourceWithDependencyInjection(Resource):
    dependency_container: Container

    @classmethod
    async def execute_handler(cls, request, request_context, handler_name, *args, **kwargs):
        resource = cls(request, request_context, *args, **kwargs)
        handler = getattr(resource, handler_name, None)
        handler_with_deps = cls.dependency_container.partial(handler)
        response = await handler_with_deps(*args, **kwargs)
        return response

    @classmethod
    def register_routes(cls, app, base_path: str = '', container: Container = Container()):
        cls.dependency_container = container
        super().register_routes(app, base_path)

Regardless, it's a nice refactor in the spirit of SRP.

@vladmunteanu
Copy link
Owner

This is a nice proposal @LeafyLappa, thanks for the PR!
I'm going to merge this, but I would really appreciate if you had some time to write a brief section in the documentation about dependency injection and how you use it.
Although I'm personally not a heavy user of dependency injection, I can definitely understand why people accustomed with FastAPI or other frameworks would be happy to have this functionality here.

@LeafyLappa
Copy link
Author

This PR does not add dependency injection per se, it only allowed me to write a simple (likely not at all scalable, extensible or even thread safe) integration for lagom and a rather quirky one at how it does its thing... storing the dependency container inside a class field, freely accessible from inside the handler. I could publish it, I feel its place would be at https://lagom-di.readthedocs.io/en/latest/framework_integrations though, and then it could be mentioned in the documentation of this project!

@vladmunteanu
Copy link
Owner

@LeafyLappa

This PR does not add dependency injection per se

Yes, that was clear.

Well, no rush with this, but I think it would be cool to see an example nonetheless.

@vladmunteanu
Copy link
Owner

@LeafyLappa The test suite found some extra white spaces in there apparently.

@LeafyLappa
Copy link
Author

Please run it again, the last commit should fix the issue.

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