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

add the ability to give OP's permission to use .blocks #1229

Closed
deathbybandaid opened this issue Oct 10, 2017 · 7 comments
Closed

add the ability to give OP's permission to use .blocks #1229

deathbybandaid opened this issue Oct 10, 2017 · 7 comments
Assignees
Labels
Declined Requests that will not be implemented for technical or project direction reasons Easyfix Bugs or Tweaks that are easy for a new contributor to fix or implement Feature

Comments

@deathbybandaid
Copy link
Contributor

deathbybandaid commented Oct 10, 2017

I did a hacky patch to make this work.

from sopel.module import OP
    whotriggered  = trigger.nick
    for c in bot.channels:
        room = c
    if not trigger.admin and bot.privileges[room][whotriggered] != OP:
        bot.say('You are Not authorized for this feature.')
        return
@dgw
Copy link
Member

dgw commented Mar 23, 2018

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.)

@dgw dgw added Feature Easyfix Bugs or Tweaks that are easy for a new contributor to fix or implement labels Mar 23, 2018
@deathbybandaid
Copy link
Contributor Author

bookmarked to look at when I get some time

@dgw dgw added this to the 6.6.0 milestone Mar 23, 2018
@deathbybandaid
Copy link
Contributor Author

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.

@deathbybandaid
Copy link
Contributor Author

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.

@dgw
Copy link
Member

dgw commented May 21, 2018

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.)

@dgw dgw modified the milestones: 6.6.0, 7.0.0 Nov 6, 2018
@dgw dgw modified the milestones: 7.0.0, 7.1.0 Nov 16, 2019
@dgw
Copy link
Member

dgw commented Nov 16, 2019

Punting minor tweak that seems to be undecided exactly how it should be implemented.

@dgw
Copy link
Member

dgw commented Apr 6, 2020

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.

@dgw dgw removed this from the 7.1.0 milestone Apr 6, 2020
@dgw dgw added the Declined Requests that will not be implemented for technical or project direction reasons label Apr 6, 2020
@dgw dgw closed this as completed Apr 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Declined Requests that will not be implemented for technical or project direction reasons Easyfix Bugs or Tweaks that are easy for a new contributor to fix or implement Feature
Projects
None yet
Development

No branches or pull requests

2 participants