-
Notifications
You must be signed in to change notification settings - Fork 809
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
feat(plugins): Add kork-plugins, wire up initial spel function extension point #3242
feat(plugins): Add kork-plugins, wire up initial spel function extension point #3242
Conversation
2b53436
to
bd5013b
Compare
import org.jetbrains.annotations.Nullable; | ||
import org.pf4j.Extension; | ||
|
||
@Extension |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this would essentially be a "system extension"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's right. Although I'm gonna delete this from the PR. I didn't mean for it to come along for the ride.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it seems that if functions can be expanded we might need a way for the extension to provide additional extraAllowedReturnTypes
for example, but we can tackle that later, obviosly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I have no intention of changing how spel function customization is done, only in providing new ways to provde said functions.
@@ -132,8 +137,8 @@ UserConfiguredUrlRestrictions userConfiguredUrlRestrictions( | |||
|
|||
@Bean | |||
public ContextParameterProcessor contextParameterProcessor( | |||
List<ExpressionFunctionProvider> expressionFunctionProviders) { | |||
return new ContextParameterProcessor(expressionFunctionProviders); | |||
List<ExpressionFunctionProvider> expressionFunctionProviders, PluginManager pluginManager) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where is the bean for PluginManager
, i am confused...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, does it come from pf4j?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it comes from SpinnakerPluginManager
which is configured in Kork
2b54fc2
to
78b90ba
Compare
78b90ba
to
f9f6a31
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Depends on spinnaker/kork#398