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

Features/middleware #2106

Closed
wants to merge 18 commits into from
Closed

Conversation

k9ert
Copy link
Collaborator

@k9ert k9ert commented Jan 31, 2023

The current implementation of callbacks has a major flaw: The callback can collect results from different extensions but it can't let extensions modify the results of former extension-modifications.

So this PR does those things:

  • Refactoring of the ServiceManager to an ExtensionManager including references from services to extensions
  • Redefining the callbacks in callback.py to enable different return_styles. The string representations has been modified to be using classes
  • Implementing a new return_style called "middleware" and the old-style will be called collect:
    • collect will return a dict where the extension_id will be used as key and the returnvalue as value.
    • middleware will assume that something will be passed to the callback as value and the returnvalue (might be the same or different) will be passed to the next callback of the next extension until the last extension's returnvalue will be returned by the execute_ext_callback-method.
    • We have some documentation here
  • The extensions might need order for that. So now every extension can specify a depends Attribute with a list of Extension-classes. The callbacks will get called so that the dependency tree and the sort-priority is respected.
  • Also we're implementing here a way for extensions to create their own callback-classes.

@netlify
Copy link

netlify bot commented Jan 31, 2023

Deploy Preview for specter-desktop-docs canceled.

Name Link
🔨 Latest commit 2390040
🔍 Latest deploy log https://app.netlify.com/sites/specter-desktop-docs/deploys/63e3807e1ff33800082bf383

@k9ert
Copy link
Collaborator Author

k9ert commented Feb 8, 2023

Closing this in favor of #2186
This should go into uiux-revamp and not into master.

@k9ert k9ert closed this Feb 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant