这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@Bnyro
Copy link
Member

@Bnyro Bnyro commented Feb 5, 2025

What does this PR do?

  • this PR changes the syntax of enabling plugins:
   plugins:
     - id: 'calculator'
       default_on: true
     - id: 'hash_plugin'
       default_on: false
  • the old syntax is still valid though
  • any plugins that are not listed in this configuration option are disabled completely

Why is this change important?

  • this PR allows instance maintainers to set whether a plugin is either
    • on by default: users can manually opt out in the settings
    • off by default: users can still enable it in the settings
  • Previously this has been hardcoded in the plugin implementation, which is bad for instances that want to provide a plugin, but not have it enabled by default (e.g. for performance reasons)

How to test this PR locally?

  • enable and disable plugins

Related issues

closes #4263
closes #759

@Bnyro
Copy link
Member Author

Bnyro commented Feb 5, 2025

I'm a bit confused about the current way we identify plugins:

  • In settings.yml, we reference them by its name, although that is defined via gettext(...) in some plugins, so that it's translatable
    • This probably will break if the instance uses a different default language? Why are we even doing this at all?
  • In the user settings (via cookies), we identify them by their id, which should probably be the only way that it's possible to reference them

@Bnyro Bnyro force-pushed the plugin-defaults branch 3 times, most recently from feb1dc0 to 37716f4 Compare February 5, 2025 13:28
@dalf
Copy link
Contributor

dalf commented Feb 5, 2025

I'm a bit confused about the current way we identify plugins

Since a long time, it is possible to reference plugin by id (python module name), the reference by user name was there as a retrocompatibility ; however the switch was never done.

This PR is good opportunity to do it.

[EDIT] the code has changed compare to what I remember, the above statement might be wrong.

@Bnyro
Copy link
Member Author

Bnyro commented Feb 6, 2025

Since a long time, it is possible to reference plugin by id (python module name), the reference by user name was there as a retrocompatibility ; however the switch was never done.

This PR is good opportunity to do it.

plugin_id is a variable that can be configured in each plugin's python file.

Many plugins don't have an ID set yet, so I assume we need to add IDs to them? Or does the ID default to the filename of the module? Isn't that ID property useless anyways, shouldn't we always use the filename as ID?

@return42 As you have recently modified the plugins logic, do you have any opinion on that?

@return42
Copy link
Member

return42 commented Feb 7, 2025

Sorry for my late response: I will look for answers to your questions later, but first I want to introduce the model behind:

TL;DR; Plugins should be identified by ID and they are identified by the ID .. there is just some old quirk around the settings option enabled_plugins / @dalf knows this better than I and I agree with his #4282 (comment)


Plugins & Answerers were revised in the course of typification. These can now be implemented as classes.

Plugins have an id:

class Plugin(abc.ABC):
"""Abstract base class of all Plugins."""
id: typing.ClassVar[str]
"""The ID (suffix) in the HTML form."""
default_on: typing.ClassVar[bool]
"""Plugin is enabled/disabled by default."""

One Plugin has implemented as a class:

class SXNGPlugin(Plugin):
"""Plugin converts strings to different hash digests. The results are
displayed in area for the "answers".
"""
id = "hash_plugin"
default_on = True
keywords = ["md5", "sha1", "sha224", "sha256", "sha384", "sha512"]

I haven't touched all plugins .. for those plugins that have not been migrated to the class model, there is a legacy wrapper:

class ModulePlugin(Plugin):
"""A wrapper class for legacy *plugins*.
.. note::
For internal use only!
In a module plugin, the follwing names are mapped:
- `module.query_keywords` --> :py:obj:`Plugin.keywords`
- `module.plugin_id` --> :py:obj:`Plugin.id`

Many plugins don't have an ID set yet, so I assume we need to add IDs to them? Or does the ID default to the filename of the module?

It is the module name for legacy builtin plugins:

self.id = getattr(self.module, "plugin_id", self.module.__name__)

Plugins are manged by the PluginStorage

class PluginStorage:
"""A storage for managing the *plugins* of SearXNG."""
plugin_list: set[Plugin]
"""The list of :py:obj:`Plugins` in this storage."""

When SearXNG starts PluginStorage.load_builtins is called.

def load_builtins(self):
"""Load plugin modules from:
- the python packages in :origin:`searx/plugins` and
- the external plugins from :ref:`settings plugins`.

The hash plugin shown above is loaded by this part:

if f.stem not in self.legacy_plugins:
self.register_by_fqn(f"searx.plugins.{f.stem}.SXNGPlugin")
continue

the other internal plugins are loaded with the legacy wrapper ModulePlugin (shown above)

# for backward compatibility
mod = load_module(f.name, str(f.parent))
self.register(ModulePlugin(mod))

external plugins are loaded by this part of the load_builtins method:

for fqn in searx.get_setting("plugins"): # type: ignore
self.register_by_fqn(fqn)


The plugin option for external plugins has been revised ... those plugins are enabled by the full qualified class name

The enabled_plugins hasn't been touched and I agree with @dalf "This PR is good opportunity to do it."

@return42
Copy link
Member

return42 commented Feb 7, 2025

The enabled_plugins hasn't been touched and I agree with @dalf "This PR is good opportunity to do it."

From an admin's point of view, but also from a technical point of view, it makes no sense to distinguish between external and internal plugins / the switch enabled_plugins can actually be omitted ..

All plugins are inheritors of the Plugin class ..

class Plugin(abc.ABC):
"""Abstract base class of all Plugins."""

And all plugins are managed in the PluginStorage ..

class PluginStorage:
"""A storage for managing the *plugins* of SearXNG."""

In the configuration, external & internal plugins should be identified by there full qualified name (ATM this is only true for externel ones):

plugins:
  - mypackage.mymodule.MyPlugin
  - mypackage.mymodule.MyOtherPlugin

We can migrate this list to a list of dict:

plugins:
     - fqn: mypackage.mymodule.MyPlugin
       default_on: true

The plugin option hasn't been set by any instance owner / we wont have downward compatibilities ..

We just have to think about a migration path for enabled_plugins and the internal plugins that are still modules today should be migrated to the plugin class ... I was planning to do that anyway to get the legacy wrapper out of the code at some point.

return42 added a commit to return42/searxng that referenced this pull request Feb 28, 2025
Before searxng#4183 a builtin plugin was *defautlt_on* when it is listed in the
"enabled_plugins" settings, this patch restores the previous behavior.

Not part of this patch but just to mentioning in context of searxng#4263:

  In the long term, we will abolish the "enabled_plugins:" setting and combine
  all options for the plugins in the "plugins:" setting, as is already planned
  in the PR searxng#4282

Closes: searxng#4263
Signed-off-by: Markus Heiser <markus.heiser@darmarit.de>
return42 added a commit to return42/searxng that referenced this pull request Feb 28, 2025
Before searxng#4183 a builtin plugin was *defautlt_on* when it is listed in the
"enabled_plugins" settings, this patch restores the previous behavior.

Not part of this patch but just to mentioning in context of searxng#4263:

  In the long term, we will abolish the "enabled_plugins:" setting and combine
  all options for the plugins in the "plugins:" setting, as is already planned
  in the PR searxng#4282

Closes: searxng#4263
Signed-off-by: Markus Heiser <markus.heiser@darmarit.de>
@return42 return42 marked this pull request as draft February 28, 2025 12:00
return42 added a commit that referenced this pull request Mar 1, 2025
Before #4183 a builtin plugin was *defautlt_on* when it is listed in the
"enabled_plugins" settings, this patch restores the previous behavior.

Not part of this patch but just to mentioning in context of #4263:

  In the long term, we will abolish the "enabled_plugins:" setting and combine
  all options for the plugins in the "plugins:" setting, as is already planned
  in the PR #4282

Closes: #4263
Signed-off-by: Markus Heiser <markus.heiser@darmarit.de>
@return42
Copy link
Member

Superseded by:

... The enabled_plugins: section in SearXNG's settings do no longer exists. There is no longer a distinction between built-in and external plugin, all plugins are registered via the settings in the plugins: section.

The implementation is similar to what has been described in this comment: #4282 (comment)

I'm a bit confused about the current way we identify plugins:

With #4538, plugins can be registered via a fully qualified class name. A configuration (PluginCfg) can be transferred to the plugin, e.g. to activate it by default / opt-in or opt-out from user's point of view.

    plugins:
      ...
      searx.plugins.hostnames.SXNGPlugin:
        active: true
      ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Hostnames plugin defaults to off even though it is enabled in the settings.yml

3 participants