python-injector / injector

Python dependency injection framework, inspired by Guice
BSD 3-Clause "New" or "Revised" License
1.32k stars 82 forks source link

Programming to an interface #149

Open domgraziano opened 4 years ago

domgraziano commented 4 years ago

First of all, thanks for the library. It's a very good to see python moving towards this practices.

I have been playing with this library in conjunction with flask-injector (https://pypi.org/project/Flask-Injector - and thanks also for that) for a couple of months now, and I believe there are some limitations, if we want to follow and applies 3 principles from SOLID in large or particularly complex code base. Particularly, if I want to respect Liskov Subistitution , Interface segregation, and dependency inversion. Some conversation was carried on here: https://github.com/alecthomas/injector/issues/123 I thought about not opening this thread at all, as it appears from there that you are not necessarily going to support that kind of behaviour, but at the same time it's probably an healthy discussion to have as what is asked there it's a very good point, and ultimately will allow me (and others) to understand in which direction this library intends to go.

What do I want to do? I want to define an interface (using an abstract class, implementing nothing in python via abc) that sets the same contract for all implementations to respect (using type hints and returns in order to be precise about the contract). After that I want to be able to pass a particular implementation to the injection in the client code via composition, and possibly at construction time, using as type declaration the interface rather than the implementation.

eg.

class NewsFeedRetriever(ABC):

    @abstractmethod
    def get_news(self) -> NewsResults:
        pass
---- 
class NewsResults:

    def __init__(self, data: dict):
        # validate data with some custom logic
        self.__data = data

    def get_data(self) -> dict:
        return self.__data
----
class NewYorkTimesFeedRetriever(NewsFeedRetriever):

    def __init__(self, url: str, user: str, password: str):
        self.__url = url
        self.__user = user
        self.__password = password

    def get_news(self) -> NewsResults:
        # use url user and password to get the news
        data = {"news": "from new_york_times"}
        return NewsResults(data)
----- 
class TheGuardianFeedRetriever(NewsFeedRetriever):

    def __init__(self, url):
        # In this case I don't need user and pass
        self.__url = url

    def get_news(self) -> NewsResults:
        # but I still implement the contract and return NewsResults
        data = {"news": "from the guardian"}
        return NewsResults(data)
----
class WashingtonPostFeedRetriever(NewsFeedRetriever):

    def __init__(self, url):
        # In this case I don't need user and pass
        self.__url = url

    def get_news(self) -> NewsResults:
        # but I still implement the contract and return NewsResults
        data = {"news": "from washington post"}
        return NewsResults(data)
---- 
class NewsFeedAggregator:

    def __init__(self, news_feeder1: NewsFeedRetriever, news_feeder2: NewsFeedRetriever):
        self.__news_feeder1 = news_feeder1
        self.__news_feeder2 = news_feeder2

    def get_news(self) -> [NewsResults]:
        return [self.__news_feeder1.get_news(), self.__news_feeder2.get_news()]

app.py

news_feeder_1 = NewYorkTimesFeedRetriever('https://www.nytimes.com/', 'someuser', 'somepass')
news_feeder_2 = WashingtonPostFeedRetriever('https://www.washingtonpost.com/')

news_feed_aggregator = NewsFeedAggregator(
    news_feeder_1,
    news_feeder_2
)

all_my_news = news_feed_aggregator.get_news()

After some months you get bored with the Washington post and you replace it with the guardian. All you need to do is

news_feeder_2 = TheGuardianFeedRetriever('https://www.theguardian.com/')

If I use bind I can always only bind one implementation! What is the best way to achieve this with Injector ?

jstasiak commented 4 years ago

Seems like `Binder.multibind()' and/or multiprovider would work for your use case:


def configure(binder):
    binder.multibind(List[NewsFeedRetriever], to=[NewYorkTimesFeedRetriever, WashingtonPostFeedRetriever]

...

class NewFeedAggregator:
    @inject
    def __init__(self, feeders: List[NewFeedRetriever]) -> None:
        ...
domgraziano commented 4 years ago

Seems like `Binder.multibind()' and/or multiprovider would work for your use case:

def configure(binder):
    binder.multibind(List[NewsFeedRetriever], to=[NewYorkTimesFeedRetriever, WashingtonPostFeedRetriever]

...

class NewFeedAggregator:
    @inject
    def __init__(self, feeders: List[NewFeedRetriever]) -> None:
        ...

@jstasiak thanks for taking the time to look into it. What if I then have 2 NewFeedAggregator, isn't true that List[NewFeedRetriever] will be always bound to [NewYorkTimesFeedRetriever, WashingtonPostFeedRetriever] so that I cannot have 2 NewFeedAggregator one with [NewYorkTimesFeedRetriever, WashingtonPostFeedRetriever] and one with [NewYorkTimesFeedRetriever, TheGuardianFeedRetriever] ? Just trying to wrap my head around how to make the most of it and if this a workable solution for me. How would you go in such case?

Thank you!

AlistairLR112 commented 4 years ago

Seems like `Binder.multibind()' and/or multiprovider would work for your use case:

def configure(binder):
    binder.multibind(List[NewsFeedRetriever], to=[NewYorkTimesFeedRetriever, WashingtonPostFeedRetriever]

...

class NewFeedAggregator:
    @inject
    def __init__(self, feeders: List[NewFeedRetriever]) -> None:
        ...

Hey there, so are you saying that multibind is only for the binding to Lists and Dicts? And it doesn't allow multiple bindings to the same interface?

jstasiak commented 4 years ago

Yes, the answer is the same as with #123: there's only one binding of a particular interface to an implementation unless you generate (and use) multiple artificial interfaces with NewType, use some custom factory code specific to your application, use multiple Injectors (and/or child Injectors which inherit parent's bindings, but there are some really sharp edges to look for there), explicitly inject various implementation types or work around this in another way.

Now that PEP 593 is accepted and implemented in typing_extensions something like Guice's @Named could be implemented but it hasn't been yet.

domgraziano commented 4 years ago

Yes, the answer is the same as with #123: there's only one binding of a particular interface to an implementation unless you generate (and use) multiple artificial interfaces with NewType, use some custom factory code specific to your application, use multiple Injectors (and/or child Injectors which inherit parent's bindings, but there are some really sharp edges to look for there), explicitly inject various implementation types or work around this in another way.

Now that PEP 593 is accepted and implemented in typing_extensions something like Guice's @Named could be implemented but it hasn't been yet.

Interesting! Maybe this thread can became a feature request? And someone can pick it up when it can? I do believe something like what is described here is needed to make this library even better!

pauleveritt commented 4 years ago

FWIW I'm a contributor to wired which comes from the venerable zope.interface tradition of abstract interfaces with multiple implementations. The wired registry is actual the z.i registry under the hood.

I bring this up because it actually works with PEP 544 protocols. Those things, and the early mypy support for them, are freaky things. I was able to make an abstract Logo component for my pages, with a different implementations looked up based on the content/resource type of the current request.

I ultimately backed off protocols because the combination with dataclasses was still raw for mypy. But I was encouraged enough that I: (a) might try to make it a formal part (with Annotated as this package has added) of wired, and (b) might throw some money into a bounty to fix some mypy stuff.

Just my two cents. If I was smart, I'd try to merge my work into this project. But the things it's doing are too freaky.

domgraziano commented 3 years ago

Yes, the answer is the same as with #123: there's only one binding of a particular interface to an implementation unless you generate (and use) multiple artificial interfaces with NewType, use some custom factory code specific to your application, use multiple Injectors (and/or child Injectors which inherit parent's bindings, but there are some really sharp edges to look for there), explicitly inject various implementation types or work around this in another way.

Now that PEP 593 is accepted and implemented in typing_extensions something like Guice's @Named could be implemented but it hasn't been yet.

Hello I've seen here https://pypi.org/project/injector/#history that there was a new release back in September but I am struggling to see what's changed, I was wondering if any work has been done or just planned in regards of this thread or there's no active plan atm to support "programming to an interface" as described above.

Thank you!

jstasiak commented 3 years ago

See the change log here: https://injector.readthedocs.io/en/latest/changelog.html. I'm not aware on anyone working on this and I don't intend to work on anything related (also it's not clear to me what really needs to be implemented in general).

domgraziano commented 3 years ago

See the change log here: https://injector.readthedocs.io/en/latest/changelog.html. I'm not aware on anyone working on this and I don't intend to work on anything related (also it's not clear to me what really needs to be implemented in general).

Hello thanks for the clarification and the link to the changlog! I wouldn't know how to better explain the problem with additional examples. You are right when you say that this issue is a repetition of #123 feel free to close this, if it helps reducing noise and you are not planning to add that functionality. Although imho in complex projects not having this level of flexibility (namely: multiple implementations of same interface) makes things quite hard and this library difficult to adopt (although that's just my experience)