errbotio / errbot

Errbot is a chatbot, a daemon that connects to your favorite chat service and bring your tools and some fun into the conversation.
http://errbot.io
GNU General Public License v3.0
3.12k stars 614 forks source link

fix: circular dependencies error when there are none #1505

Closed browniebroke closed 2 years ago

browniebroke commented 3 years ago

I have the same issue as the one reported in #1397. I'm not sure I can fix it, but I thought it would be helpful to at least write a test reproducing the issue.

Fix #1397

Implementation note

I initially had:

But this worked fine, there was no problem. It looks like the problem depends on the plugin names, I can reproduce with:

Checklist

sijis commented 3 years ago

I really appreciate you putting this together.

Could you elaborate what you mean by

It looks like the problem depends on the plugin names

Maybe its late but I'm missing but both scenarios looks similar if not identical to me.

browniebroke commented 3 years ago

I agree, the 2 cases should be equivalent, but one passes the test I wrote while the other doesn't. Since the only difference is with the names I assumed that it's playing a role into the problem.

I guess the plugin discovery system order plugins somehow related to their names. If the name ordering matches the dependency ordering, they can start, otherwise they can't.

tmcevoy14 commented 3 years ago

Hi @browniebroke any idea when this fix will be merged?

browniebroke commented 3 years ago

No idea, no, it's not really up to me at this point. I think the next step would be for a maintainer to assess whether they are happy with my code of if they have any suggestions. This is probably not the only thing on their plate, though...

In the meantime, if you want to help, I'm sure they would appreciate if you could try my version out and see if it maybe has a bug in it. You can install from source with pip:

pip install -e git+https://github.com/browniebroke/errbot.git@36920efa7818d71d4d9dc839100385f384d4c665#egg=errbot

You can put the part after pip install in your requirements file too. If you find any problem or if it works like a charm, please report back here.

sijis commented 3 years ago

@browniebroke @tmcevoy14 Would you folks be able to see if this issue persist with the latest changes on master branch?