search

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 cascade dependency plugins #1519

Closed Nov1kov closed 3 years ago

Nov1kov commented 3 years ago

Bug reproduce

Complicated plugin structure.

Parent2 
   |
   V
Parent1
    /\
   /  \----> Child2
  V
Child1

It works correctly after load on my MacBook. But didn't after deploy & run in Docker on a Linux machine. Plugin "Parent1" can't find Childs plugins, by method get_plugin.

Research

This behavior depends on a file system. Method pathlib.Path.glob works differently on Linux & Mac. Bot load plugins depending on the order of the files. PluginManager can activate plugins by direct load, or if the plugin is DependOn. So if the plugin activates by the second way, the plugin doesn't set depends list.

Merge request

  1. Rreproduced bug by unit tests.
  2. Fixed _activate_plugin_dependencies in PluginManager.
sijis commented 3 years ago

Do you think this bug and fix is also related to #1505?

Nov1kov commented 3 years ago

@sijis Hello, I'm not sure. In my case, I have exception: Plugin dependency Child1 needs to be listed in section [Core] key "DependsOn" to be used in get_plugin.. My unit test shows root of the problemm. So, I can add unit test like from https://github.com/errbotio/errbot/issues/1397, and we'll see about that.

sijis commented 3 years ago

So, I can add unit test like from #1397, and we'll see about that.

If you can, that would be extremely helpful.

Nov1kov commented 3 years ago

@sijis

  1. I Fixed my unit tests for Linux file system.
  2. Added unit test from https://github.com/errbotio/errbot/issues/1397 @jakubclark please verify your unit test. I can't reproduce the issue on my Mac. It works ok.
  3. Please rerun tests on CI.
Nov1kov commented 3 years ago

@sijis code-style fixed!

Nov1kov commented 3 years ago

Hello maintainers! @gbin @sijis @zoni what about the merge request? I must use my fork.

sijis commented 3 years ago

I should have from free cycles this weekend to look at this more in-depth.

sijis commented 3 years ago

Thank you again for the fixes. I truly appreciate the amount of testing included.