Errbot 6.0.0 fails to start with HipChat backend

gmarjanidze commented 5 years ago

I am...

[x] Reporting a bug
[ ] Suggesting a new feature
[ ] Requesting help with running my bot
[ ] Requesting help writing plugins
[ ] Here about something else

I am running...

Errbot version: 6.0.0
OS version: CentOS 7.6
Python version: 3.6.7
Using a virtual environment: yes

Issue description

ErrBot fails to start with HipChat backend.

Steps to reproduce

Start ErrBot with HipChat backend (no actual HipChat server is required to reproduce this error).

Additional info

When I try to start ErrBot it fails with the following error message:

(errbot_6) [test@testerrbot errbot]$ errbot
11:18:04 INFO     errbot.bootstrap          Found Storage plugin: Shelf.
11:18:04 INFO     errbot.bootstrap          Found Backend plugin: Hipchat
11:18:04 DEBUG    errbot.storage            Opening storage 'repomgr'
11:18:04 DEBUG    errbot.storage.shelf      Open shelf storage /home/test/errbot_6/errbot/data/repomgr.db
11:18:05 DEBUG    sleekxmpp.xmlstream.resol Could not find dnspython package. Not all features will be available
11:18:05 ERROR    errbot.bootstrap          Unable to load or configure the backend.
Traceback (most recent call last):
  File "/home/test/errbot_6/lib/python3.6/site-packages/errbot/bootstrap.py", line 142, in setup_bot
    bot = backendpm.load_plugin()
  File "/home/test/errbot_6/lib/python3.6/site-packages/errbot/backend_plugin_manager.py", line 45, in load_plugin
    raise PluginNotFoundException(f'Found more that one plugin for {self._base_class}.')
errbot.backend_plugin_manager.PluginNotFoundException: Found more that one plugin for <class 'errbot.core.ErrBot'>.

The cause of this problem seems to be that the HipchatBackend class (in hipchat.py backend module file) is derived from XMPPBackend class and not directly from the errbot.core.ErrBot class, so the PluginInfo.load_plugin_classes(self, base_module_name, baseclass) method returns the following for the HipChat module: [('HipchatBackend', <class 'errbot.backends.hipchat.HipchatBackend'>), ('XMPPBackend', <class 'errbot.backends.xmpp.XMPPBackend'>)]

sijis commented 5 years ago

I can confirm this occurs on master branch.

I'm not as familiar with the backend code so I don't have a solution to this. I do know that hipchat is based on xmpp backend and the plugin search logic matches both because of the inheritance.

I suspect the fix is somewhere in the backend_plugin_manager.py file. It's likely in the logic searching for matches.

gmarjanidze commented 5 years ago

As in hipchat.py (HipChat backend module) the XMPPBackend class (from which the HipchatBackend class is derived) is imported like so:

from errbot.backends.xmpp import XMPPRoomOccupant, XMPPBackend, XMPPConnection, split_identifier

inspect.getmembers in PluginInfo.load_plugin_classes method (defined in plugin_info.py file) returns XMPPBackend and HipchatBackend (in case when base_module_name='hipchat').

So what if we modify the following function in BackendPluginManager.load_plugin method:

def load_plugin(self) -> Any:
    plugin_path = self.plugin_info.location.parent
    if plugin_path not in sys.path:
        sys.path.append(plugin_path)
    plugin_classes = self.plugin_info.load_plugin_classes(self._base_module, self._base_class)
    if len(plugin_classes) != 1:
        raise PluginNotFoundException(f'Found more that one plugin for {self._base_class}.')

    _, clazz = plugin_classes[0]
    return clazz(self._config)

like so (filter out the plugin's classes returned by PluginInfo.load_plugin_classes method):

def load_plugin(self) -> Any:
    plugin_path = self.plugin_info.location.parent
    if plugin_path not in sys.path:
        sys.path.append(plugin_path)
    module_name = self._base_module + '.' + self.plugin_info.module
    plugin_classes = self.plugin_info.load_plugin_classes(self._base_module, self._base_class)
    # Filter out the plugin's classes so that only the classes which were defined in a
    # backend's module file are left
    plugin_classes = [p_c for p_c in plugin_classes if p_c[1].__module__ == module_name]
    if len(plugin_classes) != 1:
        raise PluginNotFoundException(f'Found more that one plugin for {self._base_class}.')

    _, clazz = plugin_classes[0]
    return clazz(self._config)

after this only the classes defined in a backend module file (which will be derived from baseclass) will be considered as the possible classes of the backend and not those which will be just imported in the backend module.

sijis commented 4 years ago

Hipchat is no longer around, so there would be no fix for this.

errbotio / errbot