punq
punq copied to clipboard
bobthemighty
Dynamic dependency resolution?
If the class wasn't registered manually but exists in the project, can we assume it's a regular registration (non-singleton) when trying to resolve it?
For example:
class MyClass:
pass
app.container.resolve(MyClass)
^ And this should work without that:
app.container.register(MyClass)
This is how Laravel's Service Container works internally, and I would like to see this feature in punq.
Actually, you can already do this.
class MyClass:
pass
container.instantiate(MyClass)
Is there a reason why that's not sufficient?
@bobthemighty I think it doesn't cover a lot of use cases.
For instance: You cannot use this to instantiate unregistered Pydantic objects. I haven't expected Pydantic support from the container itself, but you can work around this by registering the custom creation factory:
self.container.register(
service=ClientInput,
factory=lambda: self.client_input_factory.create(data=body),
)
But the main reason is that we use the container to instantiate classes similarly to other OO languages:
def __init__(
self,
client_input_factory: ClientInputFactory,
):
self.client_input_factory = client_input_factory
It always builds using logic from resolve, so it's currently impossible to type-hint an unregistered dependency via the class constructor.
Okay. I've left this project to rot, so I need to do some work to get it back to health wrt dependencies and builds, but then I'm happy to pick this up. Any chance you could provide a failing test that would work if this behaviour were in place that demonstrates your use case?
@bobthemighty Sure. I'd tinker with that myself, but you have much more experience in Python & know the codebase, so your solution should be more harmonical. I've added a draft PR with a failing test for unregistered constructor dependencies.
Out-of-the-box Pydantic support will require a lot more effort I think, so let's start with traditional container DI where there are no registered dependencies for the class.
@michael-rubel I'm a little lost on whether you still need this, or if it was an initial thought on how to handle your request injection use-case? IIRC I had a quick look back in October, but the issue is that this breaks the MissingDependency case. I'm skeptical toward it, because per Pep 20, Explicit is Better Than Implicit.
If you don't configure your application with all of its dependencies, I think that's an exceptional case, but I'm open to debate.
@bobthemighty In my understanding, if an application uses the container to handle dependencies and some of the classes request those, it should be resolved via container, thus adding container.register() for every possible dependency seems redundant. It's not about explicit or implicit. When you reach certain scale, you know basic components of your system, and know that IoC approach is utilized basically everywhere in the codebase.
Additionally, in my opinion, every dependency should be overrideable via container (including singletons, which is not the case currently). I wonder if that's related to the shared memory nature of Python (thread-safety, etc), or just overlooked in the initial implementation.
adding container.register() for every possible dependency seems redundant. It's not about explicit or implicit. When you reach certain scale, you know basic components of your system, and know that IoC approach is utilized basically everywhere in the codebase.
I disagree profoundly with this assertion. In a friendly, helpful way :)
Punq is intended to be a simple, non-magical IoC container, and auto-registration is magic. I have opened a draft PR, with one failing test, that demonstrates the issue.
In the failing test, MessageWriter is an instantiable type, even though it's only intended as a marker, and so the resolution succeeds with a broken dependency. This is unintended behaviour, and - I think - strictly worse than raising an exception.
That said, I'd be happy with an argument to Container, eg.
c = Container(auto_register=True)
c.resolve(HelloWorldSpeaker) # resolves, returning a broken dependency
Does that work for you?
Yes, that should work :)
I agree that explicit definitions are better in general, but for most developers in a huge codebase, it would be easier to build classes only touching registrations when they need non-standard definitions, like with factories or singletons, knowing that everything else is transient by default.
If you want to test out #200 I can draft a new release. I'd like to get some of the outstanding issues fixed up for a 0.8, and then probably pause while #189 gets sorted out, which requires upstream support from Pyright, and a ton of refactoring, for a 1.0.
It works fine on simple cases locally. The only thing we can add now is to pass the same auto_register argument to the child from https://github.com/bobthemighty/punq/pull/197
I think you can merge it, I will let you know if any other problem with that auto-register feature appears. We will try to utilize that heavily to test every possible use case.
@bobthemighty would you tag the changes so I can update the dependency?