Preserve The State Of Unloaded Cogs
Description
Unloaded cogs should be saved in redis, and should not be loaded after restarts.
Reasoning
From time to time, we may end up with problematic code that we choose to unload. This can happen for many reasons, from slight oversights to huge breaking bugs. The main point is, when we unload something, we usually don't want it loaded again until the problem is addressed.
Unloading right now works well enough, but it does not persist across bot restarts. There is no good way to force users to unload cogs whenever the bot restarts, so we should implement a cache on unloaded cogs.
I've been told this had been considered before, but not actioned because lancebot did not have a cache. It does now, this feature seems like a pretty important addition.
Proposed Implementation
An identifier, such as cog name should be saved in a redis cache upon unloading. During restarts, each module should be looked for in the cache, and should not be loaded if found.
Certain edge cases may occur if a module changes names while unloaded, and in such cases, it may be best to ignore it in code, and send a warning in a dev channel.
Would you like to implement this yourself?
- [ ] I'd like to implement this feature myself
- [x] Anyone can implement this feature
Cool, I will go ahead and implement this, thanks!
It's not approved yet, I want discussion from people who rejected it previously.
Certain edge cases may occur if a module changes names while unloaded, and in such cases, it may be best to ignore it in code, and send a warning in a dev channel.
Do you have any ideas for how this could work?
The bot could walk modules in the cache and verify membership in the extensions constant. This would mean looking into the cache before the bot connects, which is probably ok, as by the time the bot instance exists, we should already be connected to Redis and have a loop.
If a name is in the cache but not in EXTENSIONS, it means that it was deleted or renamed (rename probably occurs more frequently when the extension moves, changing the qualified name). In such cases, the bot could list the stale entries and offer core devs the option to prune them. It could theoretically try to correct them, but maybe that's just not worth it. However, the developer interaction cannot happen until the bot connects, so it would be deferred, and the prune handled retro-actively if confirmed. This means that you wouldn't be able to prevent a renamed extension from loading using this mechanism.
Yup, that's mostly what I had in mind. To answer a few of your points:
- Trying to figure out where a module might have been moved is not something I think is worth much time or effort.
- Any cog that should've been unloaded, but was not found will just be ignored.
- Once the bot reconnects, it'll send out a ping in some channel (preferably hidden), to alert core-devs, or anyone with permissions to unload cogs, much like defcon (used to?) does. Logging an error will have the same effect, but without a ping.
- Once alerted, a module should be removed from the cache to prevent further alerts.
Yes, this doesn't solve the problem of renamed modules being reloaded, but I think renamed are uncommon enough, and unloads are short enough in duration, that this shouldn't be a problem worth solving, just an edge case that should be handled in the simplest way possible.
Sounds good. Sending a manual alert in the desired staff channel probably makes more sense to me than logging an error, since the situation requires administrative action more so than a fix in the code.
@Shivansh-007 discussion for this issue is now closed, you are free to work on this issue if you still want it.
@Shivansh-007 discussion for this issue is now closed, you are free to work on this issue if you still want it.
Sure, I will implement this feature according to the discussion we had here and in #dev-contrib.