akatrevorjay / mainline

Simple yet powerful python dependency injection for py2/py3k
http://mainline.readthedocs.org/en/latest
Other
5 stars 2 forks source link

Detect cycles on declaration #2

Open numberoverzero opened 5 years ago

numberoverzero commented 5 years ago

This isn't very important because it'll fail loud and fast[0] but it might be nice to detect when provider dependencies form a cycle at declaration and raise a well-formed error instead of getting RecursionError on the first di.resolve("something-in-the-cycle") call.

For example, a complex graph could have a cycle that doesn't get pulled in until some heavy objects have already been loaded, which means the user partially creates some expensive resources and then crashes out[1].

Here's the repro[2]:

In [1]: from mainline import Di; di = Di()

In [2]: @di.f("rock")
   ...: @di.i("scissors")
   ...: def rock_factory(scissors):
   ...:     assert scissors == "Hi I'm scissors"
   ...:     return "Yep I'm rock"

In [3]: @di.f("scissors")
   ...: @di.i("paper")
   ...: def scissors_factory(paper):
   ...:     assert paper == "Whoa but I'm paper"
   ...:     return "Hi I'm scissors"

In [4]: @di.f("paper")
   ...: @di.i("rock")
   ...: def paper_factory(rock):
   ...:     assert rock == "Yep I'm rock"
   ...:     return "Whoa but I'm paper"

In [5]: try:
   ...:     di.resolve("rock")
   ...: except RecursionError:
   ...:     print("Ran out of turtles")
Ran out of turtles

[0] Someone would hit this situation what, once ever? [1] As a highly contrived scenario, someone setting up aws resources doesn't know about the hundreds of other tools available and rolls their own. [2] You probably didn't need this. Sorry.

akatrevorjay commented 4 years ago

@numberoverzero Hi, thanks for reporting this! Sorry I took so long to see it!

I agree that it would be nice to show the user a better error than to hit the recursion limit. I've seen this during usage a small number of times myself. It could definitely be improved!

I am leery about failing hard on a user before they attempt to actually use their recursively dependent factories. The gripe I have about raiseing at declaration time is that it's not an error until it's attempted to be resolved. (It is technically possible to overwrite a provider later (say with .update(catalog)), resolving the recursive dependency chain.)

All said, I haven't been able to come up with a simple solution to this. Any ideas?

One option is to check for this at runtime (resolve) time, using a context manager to track the current stack of executing providers (contextvars would also suffice in place of this).

[2] Examples always appreciated :+1:

Love your bottom library by the way!