-
Notifications
You must be signed in to change notification settings - Fork 1.9k
[PRODCRE-792] Support capabilities that are both Triggers and Executables #19102
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
Conversation
bb70143 to
aaa59f7
Compare
89c0710 to
ca7a685
Compare
cc59a85 to
b5609e0
Compare
core/capabilities/launcher.go
Outdated
|
|
||
| methodConfig := capabilityConfig.CapabilityMethodConfig | ||
| if methodConfig != nil { // v2 capability | ||
| w.exposeCapabilityV2(ctx, cid, methodConfig, myPeerID, don, idsToDONs) |
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.
also, shouldn't you check capability.CapabilityType==nil to make sure people not use both types of parametrizations at the same time?
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.
This type doesn't matter here any more. exposeCapabilityV2() goes method-by-method
| return nil | ||
| } | ||
|
|
||
| func (w *launcher) exposeCapabilityV2(ctx context.Context, capID string, methodConfig map[string]capabilities.CapabilityMethodConfig, myPeerID p2ptypes.PeerID, myDON registrysyncer.DON, idsToDONs map[uint32]capabilities.DON) error { |
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.
@george-dorin uncovered a very hairy bug (ticket pending...) where it seem like if the loop crashes the in-memory registry is corrupted and never recovers despite a new loop process starting.
how does this code get alerted/triggered by, say, a looop restart?
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 doesn't. And it didn't even before this change :( I opened a separate epic to handle dynamic updates to underlying capabilities and config, which should include LOOPP restarts (CRE-788)
30d2f55 to
a6bea65
Compare
a6bea65 to
7c7cebc
Compare
|
…bles (#19102) [DRAFT][PRODCRE-792] Support capabilities that are both Triggers and Executables




Launcher per-method shims and CombinedClient.
CombinedClient unit tests generated with Claude.