Per-channel configuration doesn't work as expected/documented

[dgw]

Description

It is not possible (any longer?) to use just the plugin name in per-channel configuration.

Reproduction steps

1. Set up a per-channel configuration as documented, for example:

   [#channel]
   disable_plugins = emoticons
   disable_commands = {"url": ["title_auto"]}
   

2. Start bot, and make sure it is in #channel
3. Nothing in #channel is any different from before.

Expected behavior

Per the intended functionality of this feature, and according to the website documentation, this configuration should prevent the use of all emoticons commands and block automatic link title fetching in the #channel channel.

Environment

• Sopel .version: 7.0.1.dev0
• Sopel installed via: current 7.0.x branch
• Python version: 3.6.9
• Operating system: Ubuntu 18.04 (WSL)
• IRCd /version: n/a
• Relevant plugins: n/a; this is a core issue

Notes

The problem stems from use of func.__module__ in the code that applies these restrictions:

sopel/sopel/bot.py

Lines 567 to 587 in c705f45

	# if channel has its own config section, check for excluded plugins/plugin methods
	if trigger.sender in self.config:
	channel_config = self.config[trigger.sender]
	# disable listed plugins completely on provided channel
	if 'disable_plugins' in channel_config:
	disabled_plugins = channel_config.disable_plugins.split(',')
	# if "*" is used, we are disabling all plugins on provided channel
	if '*' in disabled_plugins:
	return
	if func.__module__ in disabled_plugins:
	return
	# disable chosen methods from plugins
	if 'disable_commands' in channel_config:
	disabled_commands = literal_eval(channel_config.disable_commands)
	if func.__module__ in disabled_commands:
	if func.__name__ in disabled_commands[func.__module__]:
	return

In the past, I believe that func.__module__ used to be just pluginname for plugin callables, but that is clearly no longer the case. Now, func.__module__ for a core plugin like url is the full module path, sopel.modules.url. (It probably changed with #1479 and associated work, but I haven't verified that with a git bisect or anything. Circumstantially, #1235 implemented this long before #1479 altered the plugin mechanics, and multiple reports confirm that this used to work as documented.)

Initial implementation ideas

The immediately obvious solution is to add an attribute to each plugin callable during loading, containing the plugin name as expected by this feature's code. Presumably, being able to tell which plugin a callable came from would be useful for future features too.

Alternatively, the bot's internal tracking of loaded plugins could include a way to translate module names into plugin names. I'm partial to the first solution because retrieving an attribute of the function currently being processed is probably cheaper from a performance perspective, and logically clearer than performing a lookup on some (probably) dict stored in the bot's global state.

But either idea would work for the current version. I really don't want to wait to fix this until @Exirel's very admirable goal of refactoring plugins from "bunch of callables with attributes" to "actual Plugin object with state & behavior" happens in 8.0 or 9.0. 😅

[Exirel]

In the past, I believe that func.__module__ used to be just pluginname for plugin callables, but that is clearly no longer the case. Now, func.__module__ for a core plugin like url is the full module path, sopel.modules.url.

Sadly, this is not true. Before I replied, I read both PR #1479 and #1235, and I've to say that the bug, as described here, is actually what was implemented in the first place in #1235.

The reason is: that's how Python works. More general information about that in the Python documentation. Here is an example, taken from Sopel's source code:

>>> from sopel.modules import url
>>> url.__name__
'sopel.modules.url'
>>> url.title_auto.__module__
'sopel.modules.url'

What the code does is it looks to the function's __module__ attribute, which is set by Python, not by Sopel. It works fine for plugins defined in the ~/.sopel/plugins/ dir, because their plugin name is their Python module name, but it's not the case for these:

• sopel.modules.url: name is "url", __module__ is sopel.modules.url
• sopel_modules.url: name is "url", __module__ is sopel_modules.url
• url = my.entrypoint.plugin: name is "url", __module__ is my.entrypoint.plugin

Same plugin name, not the same __module__, and the plugin's name is not even derived from the Python module for entrypoint!

I'd argue that this feature requires a complete rework, and that your solution of adding a plugin_name to each func is probably the best one. I'd suggest to implement that not in clean_callable, but in the Plugin handler's register method.

[Exirel]

Note that if we consider that, prior to #1479, this worked for both ~/.sopel/modules/ plugins and core plugins (because they were loaded like they were in a folder and not as Python sub-module), we also have to acknowledged that this never worked for sopel_modules.* plugins.

And the lack of tests didn't help. :(

[dgw]

Note that if we consider that, prior to #1479, this worked for both ~/.sopel/modules/ plugins and core plugins (because they were loaded like they were in a folder and not as Python sub-module), we also have to acknowledged that this never worked for sopel_modules.* plugins.

Splitting hairs, I was half-right about it working before #1479 because it made core plugins behave differently from user plugins. But that doesn't help fix the bug. 😛

Per discussion here, in #1840, and on IRC, I'll work on the callbl.plugin_name solution later today. (Assuming that I don't have an unexpected run-in with Real Life™, that is. 😁)

[dgw]

Closing keyword in #1840 didn't work for some reason because this was fixed in 7.0.x, not master. Annoying, but that's just how GitHub works. Next time I'll try to remember to close the linked issue when merging.