ivankorobkov / python-inject

Python dependency injection
Apache License 2.0
672 stars 77 forks source link

Inject creates multiple objects when a singleton was requested... #16

Closed Alan-R closed 6 years ago

Alan-R commented 6 years ago

Ran into some weirdness in using Inject. Weirdness that I consider to be a bug...

I had declared that I wanted only one copy of a certain kind of object - but it gave me two distinct objects.

It took a while to figure out the symptoms in my code were caused by that, and what the underlying cause was.

One of the consumers of these objects was in a certain module, and the other was in the same file as the object declaration.

Because I was testing this new code, I had some code below the 'if name == "main" clause.

That code thought the object class was <__main__.Store>, and the other consumers (and the code that set up the injection mapping) thought it was . Although they're the same class, Python treats them as different classes... Hence, any consumers of the object which used inject.params in main, were always going to get a unique object each time it asked for one. This happens because the inject code thinks it's OK to instantiate an object (with no parameters) if there is no declared supplier of the object. In my view, that's an error. If I declare that I need a certain kind of object in inject.params, then IMHO it's an error for me not to supply it in the injection parameters.

It would have cut several hours off the process to realize what was going on sooner - with an error message. Especially if the error message said something like: No constructor for object type blah - currently known constructors are: (list of known object types). That would have made it dead obvious what was wrong immediately.

So, I think that should be an error, at least by default (maybe all the time?).

Ideally, it would be nice to figure out how to keep this from happening, then you wouldn't have to make that an error.

Here's a thought about one approach:

Upon checking, no matter where you look, cls.name gives the same answer for both main.Store and store.store. So, if you paid attention to the class name instead of the actual class, then I believe that would solve the problem.

It seems really unlikely that someone would have two different classes of the same name, and want to supply two different constructors for them.

ivankorobkov commented 6 years ago

Hi!

Python treats them as different classes. If Python treats them as different classes so does python-inject.

the inject code thinks it's OK to instantiate an object (with no parameters) if there is no declared supplier of the object Yes, it does, because it greatly simplifies the configuration. Python does not have interfaces and usually there is only one singleton implementation. You already have you graph of objects defined as @inject parameters, so no need to configure it.

if you paid attention to the class name instead of the actual class That’s a terrible hack. You see, from the interpreter point of view you DO HAVE two different classes. That’s the problem you need to solve, and not the one how python-inject treats their identity. Classes are references in python. Each reference identifies a unique class. Classnames mean nothing.

On 26 Aug 2017, at 21:49, Alan Robertson notifications@github.com wrote:

Ran into some weirdness in using Inject. Weirdness that I consider to be a bug...

I had declared that I wanted only one copy of a certain kind of object - but it gave me two distinct objects.

It took a while to figure out the symptoms in my code were caused by that, and what the underlying cause was.

One of the consumers of these objects was in a certain module, and the other was in the same file as the object declaration.

Because I was testing this new code, I had some code below the 'if name == "main" clause.

That code thought the object class was , and the other consumers (and the code that set up the injection mapping) thought it was . Although they're the same class, Python treats them as different classes... Hence, any consumers of the object which used inject.params in main, were always going to get a unique object each time it asked for one. This happens because the inject code thinks it's OK to instantiate an object (with no parameters) if there is no declared supplier of the object. In my view, that's an error. If I declare that I need a certain kind of object in inject.params, then IMHO it's an error for me not to supply it in the injection parameters.

It would have cut several hours off the process to realize what was going on sooner - with an error message. Especially if the error message said something like: No constructor for object type blah - currently known constructors are: (list of known object types). That would have made it dead obvious what was wrong immediately.

So, I think that should be an error, at least by default (maybe all the time?).

Ideally, it would be nice to figure out how to keep this from happening, then you wouldn't have to make that an error.

Here's a thought about one approach:

Upon checking, no matter where you look, cls.name gives the same answer for both main.Store and store.store. So, if you paid attention to the class name instead of the actual class, then I believe that would solve the problem.

It seems really unlikely that someone would have two different classes of the same name, and want to supply two different constructors for them.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/ivankorobkov/python-inject/issues/16, or mute the thread https://github.com/notifications/unsubscribe-auth/AAFNLm_KAFDG3qHKRsP1vpUtZiBV7lx3ks5scGi0gaJpZM4PDlf7.

Alan-R commented 6 years ago

The interpreter is treating them as separate classes, but in fact they're the same underlying class defined in the same file with the same lines of code.

Python's general attitude is "If it walks like a duck, and it quacks like a duck, it's a duck". This is clearly one of those cases where the result of combining your code with the interpreter's choices in naming the objects is in conflict with the "rule of least surprise" - and the general idea of "duck typing".

The obvious answer is simply to never give class names as the in the inject.params() call, but just use strings and be done with it. Then you can have two different implementations of the same class co-exist. For example, a logging object for security-sensitive things, and another one for messages of more normal security. Both would be logging.Logger objects. One identified by 'Secure.logging.Logger' or something similar. Of course, you could make subclasses for that - but they aren't different subclasses - they're just the same class configured differently.

So it seems that having chosen to recommend classes as the best answer for the key has some unforeseen side effects. One of the other side-effects is increased trouble with imports. If I use strings, I don't need to import the class I need for that argument, I just give a string that represents the name of that role of the argument in my infrastructure and the need to import that class disappears. If I want to have an import with constants that represent all those various roles for consistency, then I can do that. But even that won't get me into circular imports - like the recommended scheme can do.

And I also get the message I wanted for the cases where I failed to map the key into a callable function or class. So, that's what I'm going to do. So far it seems much better than using classes as they identifying key. And this is far more consistent with 'duck typing'. Now I don't care what implements the object - as long as it provides the API I'm looking for - I'm good.

ivankorobkov commented 6 years ago

OK.