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

Let callables prevent further processing #781

Closed
matthijskooijman opened this issue Apr 15, 2015 · 5 comments
Closed

Let callables prevent further processing #781

matthijskooijman opened this issue Apr 15, 2015 · 5 comments
Labels
Declined Requests that will not be implemented for technical or project direction reasons Feature Low Priority

Comments

@matthijskooijman
Copy link

As per subject, is this intentional? The code in dispatch now loops over all callables and calls whatever ones match. In some cases you might want this, but in other cases you might have fairly generic rules that shouldn't match when another module already acted on a line.

The presence of "priorities" suggests to me that only one module can act on a given message and it sounds like you should be using a higher priority for important, specific rules and and lower for more generic rule. However, in practice it seems that priorities can only be used to determine the order in which rules match. If only a single rule matches a message, priorities are effectively useless, if multiple match they determine in what order these are processed.

Would it make sense to, by default, run only one callable for each message? I can imagine that this is not always wanted so, I can imagine:

  • a decorator on a callable to indicate it should run, even if a previous callable already ran
  • a return value from a callable to indicate that the message should be further processed as if it did not match
  • a return value from a callable to indicate that processing of the message should immediately be stopped, not even be processed by callables with the decorator proposed above

Just thinking out loud here, btw :-)

@embolalia
Copy link
Contributor

Priorities are for order only. It really only matters for the few things that mutate the bot's state in some way (privilege handling, for example), and potentially for some potential performance concern (though, since everything's threaded, that'd only really matter if you were hogging the GIL for some reason).

This change would break pretty deep expectations and, since most things will just not trigger when the regex doesn't match, won't have a very big effect. It's going to require a lot of review and testing to get right. I'll leave it open, in case I ever run out of things to do, but I wouldn't expect it any time soon. (Also, semi-related is #526, which is in about the same status.)

@dgw dgw changed the title Multiple commands can match a single message Let callables prevent further processing Mar 29, 2018
@dgw
Copy link
Member

dgw commented Mar 29, 2018

Plugins in Bucket can signal to the bot that no further processing should occur on a given message, so it's not unreasonable to think that similar functionality could be built into Sopel.

It's going to be a far-future thing, and will require a look at how callables get multi-threaded since ordinarily everything gets fired off in parallel in background threads. But I'm not opposed to this idea.

@Exirel
Copy link
Contributor

Exirel commented Mar 21, 2019

@dgw for that to work, Sopel needs to stop call commands/rules using threading. There is no way to circumvent that. Sopel does not have an architecture that allows such feature.

To be honest, even though I understand the need, I don't think it suits Sopel very well. It would be a huge change, something that would have a huge impact on performances and plugin dev's expectations.

@dgw
Copy link
Member

dgw commented Mar 22, 2019

I could see leaving dispatch as-is for the most part and dropping to some sequential mode if a callable that might block others matches, but… that would still be a major architecture change.

Still, I'm not against this. But if you closed it (and edited labels "Patches Welcome" -> "Declined"), I wouldn't be sad either. Even optimistically, it certainly wouldn't happen before Sopel 9.

@Exirel Exirel added Declined Requests that will not be implemented for technical or project direction reasons and removed Patches Welcome labels May 1, 2019
@Exirel
Copy link
Contributor

Exirel commented May 1, 2019

Alright. After further consideration, I'm convince this is not they way Sopel took a long time ago, and not something that can be changed without breaking current core behaviors and existing plugins. It would make the differences between Python 2 and Python 3 small in comparison.

@Exirel Exirel closed this as completed May 1, 2019
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 Feature Low Priority
Projects
None yet
Development

No branches or pull requests

4 participants