sopel-irc / sopel

:robot::speech_balloon: An easy-to-use and highly extensible IRC Bot framework. Formerly Willie.
https://sopel.chat
Other
948 stars 402 forks source link

Alternative interval decorator that can use config settings #2632

Open dgw opened 1 month ago

dgw commented 1 month ago

Requested Feature

I envision a hypothetical @interval_lazy decorator modeled after @rule_lazy and friends. Instead of returning a sequence of regex patterns, though, the callable passed to it would return a sequence of ints. (Recall that @interval accepts multiple values—easy to forget!)

Problems Solved

Yesterday I briefly considered making the interval of a given plugin job configurable, but immediately remembered that the current decorator makes that impossible. I chose not to bodge around the limitation yet, but I will if it turns out to be important functionality.

This could also be implemented in such a way as to support another idea I saw on IRC tonight, to randomize the interval between calls. (That would probably require changes to the job scheduler, though. It's likely better if that were done with a subclass of tools.jobs.Job, and is ultimately out of scope for this feature suggestion. I did think a random interval was a neat idea, though.)

Alternatives

Right now the way to do this would be setting a fairly short interval like 30 or 60 seconds, then leaving it up to logic within the interval-triggered function to keep track of how long it's been since the last run and compare that to the relevant config setting(s).

It's not a very complicated workaround, but it feels very bodge-y. More importantly, if one expects the interval-triggered job to actually run every hour or every day at most, dispatching a plugin callable at much higher temporal resolution (60–2,880 checks per actual run in this example case) feels inefficient in the extreme. The bot's already checking for interval jobs to run every second (actually, the job scheduler is). Why add to that work?

I also thought of making the current decorator accept int | str as parameters, where the str would be interpreted as a setting name in dotted notation, or a separate decorator (@interval_setting maybe) using the same str interpretation. But I like the flexibility of a "lazy" decorator that can apply more logic than just fetching a config value—and it avoids the complexity of having to deal with unsupported setting types. ("What if the plugin tries to load a ListAttribute as an interval value, or a FilenameAttribute?")

Notes

I'm aware this is probably in the territory of "premature optimization", i.e. the performance overhead of the alternative approach is negligible. Several core and first-party plugins have frequent interval jobs. This is just an "API friendliness" kind of idea, eliminating something that plugin developers currently need to do in a roundabout, not-necessarily-intuitive, way.

And I also realize I've probably well over-explained the idea by this point. 😅

SnoopJ commented 1 month ago

This could also be implemented in such a way as to support another idea I saw on IRC tonight, to randomize the interval between calls. (That would probably require changes to the job scheduler, though. It's likely better if that were done with a subclass of tools.jobs.Job, and is ultimately out of scope for this feature suggestion. I did think a random interval was a neat idea, though.)

I mentioned on IRC that I wrote a plugin that commits implementation-detail crimes for functionality along those lines.

I think it would probably be possible to tweak the internal API so that updating .intervals on the function actually propagates to any Job objects that are downstream of it. The way it's currently plumbed, I believe the attribute only matters when the Job is created. The surprise factor when I updated func.intervals and didn't see a corresponding change in the scheduler was fairly surprising. It doesn't seem like adding some attribute access in the loop there would be prohibitively expensive, but it's also in the direction of tying Job more closely to the implementation details of a handler.

Maybe it would make sense to have a subclass of Job for plugin callables specifically (rather than any other uses of Job where the interval data comes from some other source), and the "look it up on the function" behavior could be implemented there before deferring to the base class for the common behavior.