-
Notifications
You must be signed in to change notification settings - Fork 81
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
My experience switching from haps and a multitude of questions #164
Comments
Hey @JosXa, thanks for a detailed writeup! Thank you for the kind words (I appreciate the documentation can use several improvements, even though you don't necessarily say so yourself :)). Let me respond point by point:
It depends on what specifically do you mean here. Can you show the desired API and how you'd like it to behave?
Yes, the last binding is the binding one (pardon the pun). This is kind of by design (to allow having modules that provide some defaults and then modules that overwrite some of those defaults), kind of a result of the implementation (no check has been implemented to prevent this). I appreciate this may create silent and difficult to debug issues and I'm open to accepting a patch that improves things in this regard (after some general design discussion just so everyone's time is used well). Not sure what do you mean by "polymorphic dependencies", but this behavior has nothing to do with polymorphism in my mind.
There's no verification of interactions between various objects' scopes, no formal scope hierarchy exists. This leads to a burdon on the programmer to understand those interactions and carefully craft the object graph (including the scopes) to work correctly. This can (and has) lead to sometimes difficult to debug issues and is definitely an inconvenience. Like in the point 2 above, I don't have specific plans or needs to improve this at the moment but I'm open to accepting a contribution.
I'd recommend parameterizing the library module with the implementation type to use (+ a default) like this: from typing import Type
import injector
class ICallbackStore:
def callback(self) -> str:
raise NotImplementedError("Implement me")
class MemoryCallbackStore(ICallbackStore):
def callback(self) -> str:
return "memory"
class RedisCallbackStore(ICallbackStore):
def callback(self) -> str:
return "redis"
class LibraryModule(injector.Module):
def __init__(self, store_class: Type[ICallbackStore] = MemoryCallbackStore) -> None:
self._store_class = store_class
def configure(self, binder: injector.Binder) -> None:
binder.bind(ICallbackStore, to=self._store_class)
if __name__ == '__main__':
injector1 = injector.Injector([LibraryModule])
injector2 = injector.Injector([LibraryModule(RedisCallbackStore)])
store1 = injector1.get(ICallbackStore)
store2 = injector2.get(ICallbackStore)
print(store1.callback())
print(store2.callback()) This works and typechecks with default mypy settings:
|
Sure, so let's just say I have a
Ah, my writing was not very cohesive late at night, sorry. The polymorphism here referred to named dependencies - In any case, I am having a huge issue with named dependencies and am actively looking for a nice syntax to introduce them. So with injector I went with service locator pattern instead (injecting
Yes, awesome! That works |
Another thing that seems to be missing from the docs is how injector is going to behave in the context of class hierarchies. If there's a base class that has the The field descriptor class Foo:
my_dep: Bar = Inject()
my_named_dep: Baz = Inject("a_baz") |
With regard to all these syntax suggestions that are possible with modern Python, I've been dying to pick your brain about that revolutionary new DI system that FastAPI has brought forward, I'm sure you've seen it: https://fastapi.tiangolo.com/tutorial/dependencies/#create-a-dependency-or-dependable At first glance it seems odd to "abuse" the default parameters for the purpose of function markers, but then you realize that it doesn't hurt because FastAPI/Pydantic are built around this concept that the default value is always the first argument to whatever you're replacing it with: class Model(BaseModel):
my_field: List[str] = Field(["a", "b"], min=2)
second_field: List[int] = Field(..., some=other, args=None) or @router.get("/query/list_str/default")
def get_query_type_list_str_default(query: List[str] = Query(['a', 'b'])):
return f"foo bar {query}" They can basically tick all the boxes with just these simple concepts and build arbitrarily complex hierarchies of dependencies... Do you have any opinion on this as a de-facto expert? :) |
It's been requested before (#111 and maybe in other places too), there's no way to do this right now. Your Module will likely need other Modules to be able to instantiate things, so testing one module this way, in separation, may be troublesome at best. Also unclear what "unused bindings" are in this context – they presumably can be used by the users of your library and they stop being unused?
It depends. You can test your Modules for example – create an Injector and some stub modules + your actual module, see if you can instantiate things you want in a way you want etc. There's no support for injecting into base classes' constructors, the only constructor Injector will inject into is the one constructor of the class it constructs. See commit dcbd8c4.
The only way to introduce them here is using
This seems like an interesting use of default parameters to solve the problem, but this is my only comment after reading this, I haven't actually used it. |
Yes, exactly. Let me back up a bit and provide an example: # pytest
from ticktick import * # <-- a lib I'm making
def test_module_can_be_instantiated():
inj = Injector([TickTickModule(TickTickCredentials("login", "pw"))])
for t in [ # ⬇️ manually list all the injectables from this module
TickTickClient,
Executor,
CacheSettings,
TickTickCredentials,
TickTickDataCache,
TickTick,
Queue,
]:
assert inj.get(t) # 👈 ensures in the future that no regressions have been introduced The idea would be to have a convenient way of testing that all injectables from a module can indeed be retrieved.
We're speaking more about design time than actual usage of the library with this I'd say. Unlike @pristupa I have yet to encounter a use case where I need all bindings from a(ll the) module(s) at actual runtime. Usually that should be solvable with multibindings I suppose.
Hmm, my intuition strongly disagrees with this. In my eyes a module must be a standalone unit, otherwise it has no reason to exist. As a result, I think if you're introducing cross-dependencies between modules you're breaking the unenforced rule (that exists in my head 😄) which says that from composition root down to the leaves your object graph must be a strictly directed. |
By the way, please feel in no way obligated to address all of my comment spam @jstasiak - I'm just having a good time with the library's design and wanted to have a nice chat about it, so if you feel I'm stealing your time and it's not fun or interesting to discuss for you then that's alright 😉 |
Figured out a way to have named dependencies: T = TypeVar("T")
Named = Annotated
class TestModule(Module):
def configure(self, binder: Binder) -> None:
binder.bind(Named[str, "first"], to=InstanceProvider("first_value"))
binder.bind(Named[str, "second"], to=InstanceProvider("second_value"))
binder.bind(str, to=InstanceProvider("raw_string"))
inj = Injector(TestModule)
assert inj.get(Named[str, "first"]) == "first_value"
assert inj.get(Named[str, "second"]) == "second_value"
assert inj.get(str) == "raw_string" Very cool that it worked out of the box 👍 I think that would deserve a PR with at least the |
There are design decisions to be made before something implementing named dependencies is merged. The code above works by accident and only in some cases. Try adding this and it'll break: class X:
@inject
def __init__(self, first_str: Named[str, "first"]) -> None:
self.first_str = first_str
assert inj.get(X).first_str == "first_value" Since Since this is adding a layer of complexity I think we need a set of test cases first, but I don't have time to provide it right now. |
Having a special class provided by injector should solve the issue: assert inj.get(Annotated[str, injector.ByName("first")]) == "first_value" frameworks should ignore all metadata provided in |
Hi @jstasiak and the team,
Last week I went on a research spree to get myself updated on what's the latest DI/IoC hotness nowadays, after having
made an initial decision to adopt haps as my go-to library two years ago. This is how I stumbled over injector and I might aswell have snorted that documentation through my nose that's how excited I was about it.
The way it looks, I'm gonna make the switch and go on using this well-maintained and stable alternative going forward.
So during the past days I started porting some code over and it went fairly smoothly and without many surprises, but
still there were a bunch of uncertainties and lack of guidance with regard to what design patterns should be used
when it comes to injector.
I would like to bundle those concerns in this issue to make writing this essay easier (😄), but would also be happy to split them up into separate issues if you pointed out candidates worth doing so. The goal with this issue is mainly to get some usage questions answered, but also to start a conversation around the state of dependency injection in Python and how injector fits in
that ecosystem.
All of the following is in the context of "modern Python" with full typing - I have absolutely no interest in coding in or supporting anything below 3.7.
1. [Usage Question] Is there are way to force validity checks on modules and
configure
-functions at startup?During my experiments I was missing something like a
binder.bind(..., eager=True)
and a way to enforce this on module level also. I thought that one of the advantages of dependency injection should be increased safety from runtime problems, and if I could just invoke all bindings in a module with a single test, I figure that would help.2. [Oddness] Bindings get silently overridden. Are polymorphic dependencies an intended use case?
Haps does not allow you to configure more than one implementation for a given base type, but for injector that doesn't seem to be a problem. At least on the surface, the following code passes without warning:
If I now attempt to
get
anIFoo
out of an injector, from what I can tell the library just silently overrides whatever is already configured with the most recent declaration. Is this intended? I haven't seen the expected behavior documented anywhere, so maybe you can shed some light on whether and how to use this kind of polymorphism.3. Strict Scopes
From C# .NET Core and Microsoft's builtin dependency injection framework, I am used to scopes having a special meaning (in .NET) and the requirements/scope hierarchy being more tightly controlled when it comes to mixing different scopes with one another.
For example, they will throw an exception at you for using an instance-scoped type as part of a singleton-scoped class, as this would cause the former to lose its integrity of "being instantiated anew on every invocation".
This essentially forces you to understand and think about proper scoping and your dependency graph, so it results in
cleaner architecture.
From what I can tell, scopes in injector don't bother with any of these shenanigans, but I was curious what your thoughts are about this.
4. [Best Practices] How to work with injector to expose functionality as part of a library?
Most of my Python coding goes towards a variety of libraries that make building and working with bots on Telegram Messenger easier. This question concerns a rather complex framework called Botkit and the ability to modularize both the internals and the public-facing interface, as well as to expose a convenient API for users to split their code into logically-sound modules/components/"reusables".
Using haps, I ran into many problems here due to its global nature and the fact that it's not really meant for hierarchical organization of modules, at least not nearly as nicely as the explicit container declarations that injector offers.
Now, using injector, I ran into a problem related to 2. above. Essentially, I would like to offer different implementations of an abstract
IFoo
(some kind of service), but already expose one default implementation from a prebuilt module.What would be a concise way to let users decide they want to use a different implementation, and in which way would
you recommend they should pass required dependencies for their particular choice into the framework's setup code?
Here is the concrete example:
Library offers
IDataStore
and a reference implementationMemoryDictDataStore
, but also aRedisDataStore
that would require an extra
pip
-installedredis
module; but also aSQLAlchemyDataStoreImpl
with its ownoptional installation. Assuming the dependencies for all three implementations are passed to the library (somehow), how could
you afford to the user to make a decision with a nice API?
Using
haps
, I was able to solve this in a very elegant way using named dependencies, and my approach was to expose something likemy_lib.use_data_store_impl: Literal["memory", "redis", "sqlalchemy"] = 'memory'
and it would change the implementation at runtime, but neither does injector seem to offer named dependencies, nor would this way of going about it fit with its general style, I think...Haps implementation:
I will end here for tonight, but there are still 3 more things on my list that I might address depending responsivity and time :)
Thanks in advance!
The text was updated successfully, but these errors were encountered: