Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Separate config section definition from bot setup #2597

Open
dgw opened this issue Mar 10, 2024 · 0 comments
Open

Separate config section definition from bot setup #2597

dgw opened this issue Mar 10, 2024 · 0 comments
Labels
Core/Plugin Handling Feature Long-term Planning Things that need to happen at some point in the future, but need to NOT happen soon.
Milestone

Comments

@dgw
Copy link
Member

dgw commented Mar 10, 2024

Plugin authors must currently remember to define_section() in two plugin hooks:

  1. setup(), so the plugin's configuration is registered when Sopel runs
  2. configure(), so the plugin's configuration is registered when the interactive config wizard runs

This is a minor inconvenience, true, but it leads directly to a larger problem: Plugin configuration definitions aren't available outside of these two scenarios. If the bot isn't in the config wizard, and isn't running normally, option validation is unavailable.

Implementing command-line sopel-config set functionality (as I started doing a while ago in #1986) becomes tricky, therefore, if we want the CLI to be safe and reject invalid values (such as options not available in a ChoiceAttribute). Plugins' StaticSection definitions have to be available to the CLI, but running a plugin's setup() can come with side effects.

I'm sure we can just setup() plugins before attempting to handle a hypothetical set or unset CLI action, and things would be fine most of the time. I think we can do better long-term, though, and that's what this issue is about: planning for the long term.

In some minor release (hopefully within 8.x), we can add a new way for plugin authors to register their config sections that isn't tied to the plugin's setup() or configure() hooks. It will have to be implemented such that the relevant submodules can do (pseudocode):

if new_method.exists():
    new_method.use()
else:
    old_method()

But assuming we can devise a sensible mechanism, the new and old ways of defining a config section can coexist until the next major Sopel version when we'd remove Config.define_section() from the API.


Considerations

We are theoretically trying to avoid reserving new top-level object names. The simplest way to do so is making any StaticSection subclass auto-loaded, but the catch is that a StaticSection on its own doesn't have a name; the name used in .cfg files comes from the call to Config.define_section().

However, that leaves us with a natural fallback point: Auto-register any StaticSection subclass that has been given a name as part of its definition (using a yet-undetermined mechanism), and leave Config.define_section() to register and name existing subclasses (until the plugin author updates to the new style).

I admit that I haven't had time to think this specific implementation idea all the way through, but it's also just fine to get a gut reaction from the other maintainers when we start on 8.1 development and figure out the path forward together.

@dgw dgw added Feature Long-term Planning Things that need to happen at some point in the future, but need to NOT happen soon. Core/Plugin Handling labels Mar 10, 2024
@dgw dgw added this to the 8.1.0 milestone Mar 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core/Plugin Handling Feature Long-term Planning Things that need to happen at some point in the future, but need to NOT happen soon.
Projects
None yet
Development

No branches or pull requests

1 participant