-
Notifications
You must be signed in to change notification settings - Fork 38
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
Simplify event system #165
Comments
#144 was another bug caused by this issue. |
Hello. I've found this page from the "good first issue" tag, and I do not have any experience in contributing to an open source project. I'd like to contribute to fixing this bug, but I don't understand parts of the issue, like "argspec", and how the event callbacks work in general. For now I will try to take a look on some of the files, but any help will be greatly appreciated. |
I've checked plugins.py and my understanding is that: Is this correct? |
When I created the event system, I really just wanted a way for plugins to be able to subscribe to a data stream from other plugins, and ideally signal a message back to them. This was done for the URLs plugin so it could allow other plugins to override the URL title fetching behavior.
At the time, I wanted to provide some additional guarantees when loading a plugin that the event callback was invalid if the argspec didn't line up with the event emission. While this was admirable, I have actually noted that when using Cardinal, I tend to miss any error messages that occur during the initial plugin loading when Cardinal starts up to do all the noise in the logs. It may actually be beneficial to get an error in the logs every time that the callback is attempted to be called - it would certainly make the issue more obvious.
There are other downsides to the current approach as well - callbacks are unregistered when the event is unregistered, which results in bugs such as #143, and to a lesser extent, #142. Furthermore, the callback that ends up being registered isn't well-defined as far as its name is concerned. We refer to these with "callback IDs" which aren't very useful in the logs. If we'd like to configure events to be disable-able, we'll want them to be more addressable as ewll.
So instead, we'll make event callback subscription very lightweight - the event need not exist, and no validation is performed. Any plugin will then be able to send any amount of data via its channels.
The text was updated successfully, but these errors were encountered: