Closed deathbybandaid closed 4 years ago
This could make a good feature PR for 6.6.0 if you're interested, @deathbybandaid. (Pro tip: use >= OP
instead, so channel owners and protectops can access it too.)
bookmarked to look at when I get some time
I was taking a look at this,,, what are your opinions on leveraging the database in some way for more granular control?
In my instance of sopel, commands must run trough a "preflight checklist" before running. This was decided for several reasons for the channels the bot is in.
Things covered in that list (some of these are circumvented by running in privmsg to the bot):
Check blocklist in database for the channel the command is run in. (instead of just blocking, block user from using bot in specific channel)
Is the module's command allowed in that channel (I have all modules disabled by default, and an admin command can enable/disable them)
My bot is also opted-out by default, and a specific command opts you in.
If these criteria are met, then the module will run.
I think the bot (as a whole) could benefit from a .botadmin
command with subcommands like .blocks
.
I think blocks should then utilize the database for channel specific blocks.
I'm happy to use the database for permission storage, so any changes persist across restarts of Sopel, but the last thing the database needs right now is more queries. We already run into concurrency problems with the current set of DB-bound operations on busy channels and/or slow hosts (see #736) because SQLite is not great at handling concurrent accesses from multiple threads. For as long as SQLite remains Sopel's primary (and only, currently, but see endnote) supported database type, we should definitely try not to increase the load on the DB too much.
So, right now I'm of this opinion: If such a system gets built out it must primarily use a data structure in bot.memory
to reference permissions during runtime. You can look at how Bucket (the #xkcd bot) handles caching text variables with less than 1000 (by default) values, for example. The whole section is kind of long, and Bucket's code is mostly one giant if-elseif tree (and it's in Perl, too…), but these lines are what actually stores the value once the bot validates that it isn't a duplicate, etc.
I think a similar approach can work with what you're proposing, especially since Sopel already has a "memory" implemented. (Whether sopel.memory
itself can or should take care of persisting to the DB is up for debate, since some uses will need persistence and some won't.) Changes would generate database queries, and Sopel could load the existing configuration from the DB on startup, but the vast majority of Sopel's life cycle would be spent just using its cache in memory.
It's also worth pointing out that a system for (en|dis)abling modules and commands on a per-channel basis is already in the works. In #1235, @lenisko dropped in a config-based approach. Assuming it ships in 6.6.0 as currently planned, that would leave Sopel 7 wide open for big changes like adding back support for MySQL/PostgreSQL via SQLAlchemy (as proposed by @cerealcable in #736)—which in turn would make it less risky to add features that put more load on the database, like this proposal.
(Yes, this issue too is marked for 6.6.0 currently, but if we do go with this database thing it'll get bumped.)
Punting minor tweak that seems to be undecided exactly how it should be implemented.
I'm going to make a decision: We shouldn't do this. Anyone who needs to manage blocks should be added to Sopel's admins list, rather than letting any chanop block people from using a bot across the whole server.
I did a hacky patch to make this work.