No context is created when no injectables are found, attempt to put object - fails #triage @bug
mt3o opened this issue · 4 comments
When we create a new context with load_injection_container()
and there are no injectable objects found, no namespace is being created in injection_container.py
. When we attempt to programmatically add an object into the context - it fails - because no default context (namespace) has been created.
Code to reproduce:
from injectable import Injectable, load_injection_container
from injectable.testing import register_injectables
class Sample():
pass
def fail():
sample = Sample()
mocked_injectable = Injectable(lambda: sample)
register_injectables({mocked_injectable}, Sample)
print("it's broken")
def run_example():
load_injection_container()
fail()
if __name__ == "__main__":
run_example()
Response:
File "sandbox/fail.py", line 12, in fail
register_injectables({mocked_injectable}, Sample)
File "\.virtualenv\lib\site-packages\injectable\testing\register_injectables_util.py", line 54, in register_injectables
namespace = InjectionContainer.NAMESPACES[namespace or DEFAULT_NAMESPACE]
KeyError: 'DEFAULT_NAMESPACE'
In this case - when we try to register_injectables
, we see KeyError
on attempt to get default namespace - when there is none.
How to fix:
When container fails to find any injection candidates - it should create empty default namespace anyway.
Hi @mt3o! Thank you for the issue with a reproducible minimum example
I can confirm that this is a bug affecting injectable 3.4.4 and all prior 3.x releases. While I work on the fix the suggested workaround is to invoke load_injection_container
on a path it will find at least one injectable class to force it to create the namespace and then you can use clear_injectables
to remove any unwanted dependencies registered.
I will keep you posted on when the fix is released.
Hi @allrod5!
I discovered another scenario related to the bug:
load_injection_container('../..') # points to valid source of objects
reset_injection_container()
register_injectables({ Injectable(lambda: make_context())}, GlobalContext)
The same bug appears. Not calling reset_injection_container
but calling clear_injectables
helps.
Regardless, I have to say, you did an awesome piece of work with this library. The DI from ets labs is quite complicated, imho you found the balance between simplicity of code, ways to use, and functionality. My app instantiates several dozens of objects and calle them when they match a protocol - so I heavily rely on injecting lists of items that match the assumed type. I tried using libraries that provide js-like events, but they lack the stopPropagation()
, preventDefault()
and isDefaultPrevented()
methods that are present in js, and I need each object from the list to be able to break the chain of commands.
Do you think it would be possible to inject items that do not inherit the given protocol class (as instantiable classes should not subclass Protocol
) but match a protocol on runtime? Could it be a feature request? :)
Hi @mt3o, thanks for your feedback ❤️ and for sharing your thoughts.
Update on the bug: the fix is really easy and is on the way!
Regarding this other piece of code that reproduces the bug, there is really nothing different from the original code actually because when you can reset_injection_container
you're not just clearing injectables from a namespace but wiping clean the whole container so all namespaces are being trashed as well just like as if load_injection_container
was never called. I can see that maybe you were expecting reset_injection_container
to behave similarly to clear_injectables
but clearing all injectables at once. Once this bug is fixed I see no strong reason to create a clear_all_injectables
utility with such behavior but I'm open to discussing that if anyone comes up with a good use case 🙂
About the feature request, I'm not that familiar with the js events system so a more concrete example would help me there. I think it would be best to discuss this possible feature in a separate GitHub issue though.
Hi @mt3o, just passing by to inform you that the fix has been released in version 3.4.6 and already is available at PyPI. Thanks for your help on this bug! 🎉