issue with plugin importing logic

[amorfinv]

TLDR: there is an issue with importing plugin logic when plugins import other plugins because they may get saved into sys.modules dict with a key of plugins.plugin instead of plugin which means that they may be loaded twice into memory even with the latest check from commit 1392545. Solution could be to save all plugins into sys.modules dict with a key standard of plugins.plugin however I am unsure if it would work when plugins folder is located somwhere else (like when using it as a pip package).

---

Hello,

It seem the issue with importing plugins from other plugins is back. If we repeat the example from #366 seen here in which the following plugins are enabled:

enabled_plugins = ['ssd', 'geofence']

Doing

from plugins import geofence inside SSD plugin gives a

Attempt to reimplement GEOFENCE from <function geofence at 0x137d398b0> to <function geofence at 0x137d35940>

message.

It seems like it is related to how plugins get loaded in the pip package (commit 2e85157). in lines 46-58 of bluesky/core/plugin.py

# Load the plugin
# First check if plugin is already locally imported by another plugin in the same directory
# In either case update the other import name to avoid double imports
self.imp = sys.modules.get(self.module_name)
if self.imp:
    sys.modules[self.module_imp] = self.imp
else:
    # self.imp = importlib.import_module(self.module_imp)
    # sys.modules[self.module_name] = self.imp
    spec = importlib.util.spec_from_file_location(self.module_name, Path(self.module_path) / (self.module_name + '.py'))
    self.imp = importlib.util.module_from_spec(spec)
    sys.modules[self.module_name] = self.imp
    spec.loader.exec_module(self.imp)

The last line spec.loader.exec_module(self.imp) executes the plugin. If the executed plugin (A) imports another plugin (B) then plugin B is also executed unless it is already inside sys.modules. If plugin B is already inside sys.modules then plugin A just gets a reference to plugin B. I think this is what is intended in the if statement above in commit 1392545. However, it does not seem to work anymore for the example of loading geofence into ssd.

If we follow the import logic

1. ssd is imported without any problems. However when spec.loader.exec_module(self.imp) runs then geofence is loaded into the sys.modules dictionary with the key "plugins.geofence".
2. Now when it is the turn of geofence to be loaded self.imp = sys.modules.get(self.module_name) returns None because it searches for the key geofence rather than plugins.geofence. This means that geofence passes to the else and gets loaded into memory twice which causes the Attempt to reimplement GEOFENCE

---

I tested a solution for this by also checking insidesys.modules for modules that were loaded with plugins prefix . For the example above we check for plugins.geofence if self.imp initially returns None. See here.

The above code snippet now looks like this and I also have a class variable called self.relative_name that would save the path of each plugin as "plugin.*" so ssd would be "plugin.asas.ssd"

self.imp = sys.modules.get(self.module_name)

# also check if the plugin has been imported by another plugin in the same directory
if self.imp is None:
    # In all modules search for relative import from the relative_name             
    if [plugin_module for plugin_module in sys.modules if self.relative_name == plugin_module]:
        self.imp = sys.modules[self.relative_name]

if self.imp:
    sys.modules[self.module_imp] = self.imp
else:
    spec = importlib.util.spec_from_file_location(self.module_name, Path(self.module_path) / (self.module_name + '.py'))
    self.imp = importlib.util.module_from_spec(spec)
    sys.modules[self.module_name] = self.imp
    spec.loader.exec_module(self.imp)

With this change the above example would now work but now it is heavily dependent on the order of how plugins get loaded into BlueSky.

If I now switch the order to

enabled_plugins = ['geofence', 'ssd']

then the Attempt to reimplement message reappears. If we look at the importing logic again

1. geofence goes into the else statement and is loaded without any issues and does not import any other plugins. It is saved with key geofence inside sys.modules.
2. ssd was not loaded by geofence so it also goes into the else. So when it gets to spec.loader.exec_module(self.imp) the code inside ssd is executed and when it gets to from plugins import geofence it first checks plugins.geofence is inside sys.modules . However in sys.modules the key is geofence. This then causes an Attempt to reimplement.

If I change the import statement inside ssd to import geofence then it seems to go ok.

However, I can imagine cases where you may have organized your plugins into several folders and need to use from plugins import plugin statements. So I am not sure what the correct solution to this. Perhaps all plugins should first be loaded into sys.modules with the key "plugins.plugin". Once they are all in sys.modues you can then run spec.loader.exec_module(self.imp) to load the plugin. But I am not sure if this would work if the plugins folder is somewhere else like how it would be for when using the pip package.