Automatic reloading of PyFilePlugins

[hftf]

I suppose it should be enabled by a config flag, since this is mostly useful for developing and testing new modules, but not very useful in production when modules are assumed to be stable.

---

Maintainer note: We should only support auto-reloading single-file plugins. Not even manually reloading package or entrypoint plugins works properly, so they definitely cannot be auto-reloaded. Scope of this feature is to add a config option that enables auto-reload for single-file plugins only.

Jury's still out on whether it should be a Boolean or a list of plugin names. Either, really. There's definitely a case to be made for whitelisting the specific plugins that can be auto-reloaded. — @dgw

[elad661]

I don't think it's a good idea, it would cause too much reloading when I'm working on a module with auto-saving enabled...

[dgw]

@elad661 That's why it would be enabled by a config flag.

[embolalia]

I'd prefer an option to the command, but yeah. It would be optional.

[Exirel]

I wonder if a inotify script would do the trick, without having to add anything to sopel's code.

Like, sopel --quit && sopel shouldn't be too complicated to launch from inotify!

[dgw]

Or just sopel --restart when #1333 is merged in?

I'm not against adding a new command-line flag, either.

Maybe Sopel could support a --watch mode that watches modules for changes during development, along the lines of bundle exec jekyll server --incremental or webpack's watch: true config setting.

Alternatively (or additionally), sopel --reload could become a thing at some point. That could be tricky UX-wise because such an option is often used in e.g. HTTP servers to reload a config file, not plugins, but it's a thought. Might happen, might not.

[Exirel]

This conversation reminds me that I wish #1429 and #1434 to be merge before working again on the run_script file, or anything that is related to the command line. Right now, as a user and a third-party dev, I'm not very happy with the toolings around command line, and there are a lot of obscure behavior here and there in this file.

That's why, even though I'm not against a --restart/reload command, I would suggest a low priority on that.

[Exirel]

@dgw after further consideration, I don't think it's reasonable to plan this for Sopel 7.0, and probably not for the Sopel 7.x branch anyway. I'd suggest to remove the milestone for now.

[dgw]

Mm, implementing this was predicated on fixing the issues with reloading. And we've had this discussion several times now—we can't properly fix reloading until we at minimum no longer support old versions of Python (2.7, 3.3) that don't support the internal machinery needed for better reloading support.

There might be other things we need to do, too. It's interesting that you revived this particular issue today, because I was just working on a new feature for our Twitter plugin and had reloading issues. That is, telling Sopel to reload it didn't work. Obviously #1056 is still a problem. I should just bump its milestone along with this issue's, since they both ultimately depend on better reload machinery.

What does work right now (mostly) is reloading single-file plugins, i.e. ~/.sopel/modules/pluginname.py. We could implement a way to auto-reload only those to ease hacking together a new plugin, without supporting packages or entrypoints. From IRC discussion, I think there's already general agreement that this is the scope of the feature, so I'll update the OP a bit.

[dgw]

Agreed with @Exirel. We'll leave this for after we drop old Python versions, in 8.0, when we'll have guaranteed access to some better reloading machinery from more recent Pythons.

[Exirel]

I'm thinking about this feature, and I think we could drop it from any milestone, and just left it here for anyone to pick and grind their teeth on. From my perspective, this is a lot of work for very little reward, and I'd rather focus on other things for now. After all, it's a feature from 2015, and I'm not even sure it is still valid, even more now with the built-in pytest plugin with all the fixtures that make live testing less necessary.

[dgw]

I imagine this still would be useful while prototyping stuff. Not everyone starts right in on a full package with tests when creating a new plugin. We'll likely never eliminate the utility of directly reloading files.

As far as the amount of work required, how about this proposal using a library (so no need for us to maintain file-watch code):

• Implement this as a CLI option (sopel --watch) that monitors SOPEL_HOMEDIR/plugins for changes
• Build the feature on top of watchdog, and make that dependency an extra (pip install sopel[watch]) so normal users don't have to pull it down

Without having gone too deep on this just yet, I think this would require only having an event handler function that can map the path to a plugin loaded by the bot, and setting up the Observer during startup if the option is enabled. (Cue reasons why it's not "only", i.e. not that simple. 😅)

[Exirel]

I imagine this still would be useful while prototyping stuff. Not everyone starts right in on a full package with tests when creating a new plugin. We'll likely never eliminate the utility of directly reloading files.

I agree, and that's why I suggest that we remove the milestone, because I don't think it is worth it for version 8.0 per say. Nothing new is planned on the plugin system for the foreseeable future, so I guess any version will do.