Add standalone registrysyncer package #13668
Conversation
cedric-cordenier
commented
Jun 24, 2024
•
cedric-cordenier
commented
- Create a standalone registrysyncer package which accepts arbitrary handlers (launchers). To be used by CCIP and Keystone.
- Create a capabilities launcher which handles state updates for the Keystone case.
- Some cleanup: combine syncer + reader now that the syncer is significantly smaller, move LocalNode from the syncer to the workflow handler, since it contains Keystone-specific logic.
cedric-cordenier
force-pushed
the
registrysyncer-pkg
branch
from
8f4808f to
94437b1
Compare
cedric-cordenier
force-pushed
the
registrysyncer-pkg
branch
from
94437b1 to
2035275
Compare
- And workflow handler implementation for keystone-specific logic
cedric-cordenier
force-pushed
the
registrysyncer-pkg
branch
from
2035275 to
c639364
Compare
cedric-cordenier
changed the title
cedric-cordenier
force-pushed
the
registrysyncer-pkg
branch
from
cf0a407 to
8bec394
Compare
cedric-cordenier
force-pushed
the
registrysyncer-pkg
branch
from
8bec394 to
9cb117c
Compare
core/capabilities/launcher.go
Outdated
| // TODO: this is a bit nasty; figure out how best to handle this. | ||
| if len(myWorkflowDONs) > 1 { |
There was a problem hiding this comment.
This is no longer necessary because it is ensured onchain, right?
There was a problem hiding this comment.
I think we do still need some variant of this check (an error since it shouldn't happen) because the DONs are returned as a list
There was a problem hiding this comment.
I've reworded it though to make it clear that this is an invariant violation
|
| return State{IDsToDONs: idsToDONs, IDsToCapabilities: idsToCapabilities, IDsToNodes: idsToNodes}, nil | ||
| } | ||
|
|
||
| func (s *registrySyncer) sync(ctx context.Context) error { |
There was a problem hiding this comment.
Naming nit: when I was reading this, I expected to only see state sync inside the sync method, but this method also included notifying the launchers, which was a bit confusing.
I would move the "launching" part to a separate method called "notifyLaunchers". Also, launchers aren't being "launched", but rather "notified" of the new state, as this happens on a loop. FWIW, I liked your "handlers" name better (I know you changed it 😅); another alternative would be "listeners"/"addListener".
| if len(s.launchers) == 0 { | ||
| s.lggr.Warn("sync called, but no launchers are registered; nooping") | ||
| return nil | ||
| } |
There was a problem hiding this comment.
I like this. I would even take this a step further and not start the sync loop if there are no launchers (I'm assuming these aren't going to be added dynamically anyway). It seems like an error to me if we have the registry with no listeners 🤷♂️
