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

feat: naive Factory implementation for getting rid of lambdas #163

Merged
merged 8 commits into from
Sep 29, 2023

Conversation

ShahriyarR
Copy link

Description:

When I was trying to add DAOs as a dependency, Inject requires the lambda or partial as described here:

dependencies={
            "user_dao": Inject(lambda: UserDAO()),
            "article_dao": Inject(lambda: ArticleDAO()),
            "post_dao": Inject(lambda: PostDAO()),
        },

Implemented naive, but yet clean wrapper called Factory to inject as:

dependencies={
           "user_dao": Inject(Factory(UserDAO)),
           "article_dao": Inject(Factory(ArticleDAO)),
           "post_dao": Inject(Factory(PostDAO)),
       },

For now, it only supports variable arguments(*args)

Mostly inspired by:
https://python-dependency-injector.ets-labs.org/providers/factory.html

Information:

  • All tests are passed in the local run
  • Added more parameters to the original existing parametrized tests

In the future possible TODOs:

  • Add **kwargs support
  • Add dynamic argument additions such as: obj = Factory(UserDAO); obj.add_args(*args)

@tarsil tarsil changed the base branch from main to v2 September 20, 2023 22:31
@tarsil
Copy link
Collaborator

tarsil commented Sep 20, 2023

@ShahriyarR this idea looks very interesting and indeed what we have discussed. Do you think you are able to also update the docs for this with examples and how tos?

@ShahriyarR
Copy link
Author

@tarsil yes, I will add docs as well. I also need to add more tests for checking Route Level DI or Handler Level, I am still not sure how we call it :)

@tarsil
Copy link
Collaborator

tarsil commented Sep 21, 2023

@tarsil yes, I will add docs as well. I also need to add more tests for checking Route Level DI or Handler Level, I am still not sure how we call it :)

So, we have dependencies from the Esmerald instance down to handler (get, post, put....). So that means, Esmerald, Include, Gateway/WebSocketGateway and the handlers in general 🙂

You can see that also in the Esmerald tests 👍🏼

@tarsil tarsil changed the base branch from v2 to main September 22, 2023 19:36
@tarsil tarsil changed the base branch from main to v2 September 22, 2023 19:36
docs/dependencies.md Outdated Show resolved Hide resolved
@ShahriyarR ShahriyarR force-pushed the feat/factory-class-for-di branch from d66a7e7 to f9cbf59 Compare September 22, 2023 19:47
Copy link
Collaborator

@tarsil tarsil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The examples are actually really great and simple to understand. Apologies for the comments in the documentation but it is more to be consistent with the way we try to provide the "guide" through those same docs. As mentioned before, it is more like a story telling but with structure :)

docs/dependencies.md Outdated Show resolved Hide resolved
docs/dependencies.md Outdated Show resolved Hide resolved
docs/dependencies.md Show resolved Hide resolved
docs/dependencies.md Outdated Show resolved Hide resolved
docs/dependencies.md Outdated Show resolved Hide resolved
docs/dependencies.md Outdated Show resolved Hide resolved
docs/dependencies.md Outdated Show resolved Hide resolved
class Factory:
def __init__(self, provides: "AnyCallable", *args: Any) -> None:
self.provides = provides
self.__args: Tuple[Any, ...] = ()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't this be:

self.__args: Tuple[Any, ...] = args

?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be done directly, but personally I prefer to use the getter and setter way of dealing with internal objects:

        self.__args: Tuple[Any, ...] = ()
        self.set_args(*args)

@tarsil
Copy link
Collaborator

tarsil commented Sep 29, 2023

Looks good to me 👍🏼 . Good stuff @ShahriyarR . We can grow this functionality even further

@tarsil tarsil merged commit 07b2969 into dymmond:v2 Sep 29, 2023
5 checks passed
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.

3 participants