[OPCORE-863]: fix(service): start multiple feeds managers #14197
Conversation
graham-chainlink
force-pushed
the
ggoh/OPCORE-863/multiple-feeds-manager-start
branch
from
9752380
to
ca18f96
Compare
graham-chainlink
force-pushed
the
ggoh/OPCORE-863/multiple-feeds-manager-start
branch
3 times, most recently
from
b54d3e3
to
f51fd98
Compare
There was a problem hiding this comment.
Should we make this part of the change last? Could potentially cause issues as the UI and all other parts of the code can't handle multiple managers. How does this affect how the UI picks which manager to use? Maybe we should add a feature flag to the toml config first
There was a problem hiding this comment.
Discussed and decided we can add the flag when this is changed as the changes here alone does not actually start multiple fms
graham-chainlink
force-pushed
the
ggoh/OPCORE-863/multiple-feeds-manager-start
branch
from
a4b0fdf
to
b1affc7
Compare
In order to support connection to multiple feed managers, when on start of the node, we want to attempt to connect to list of registered feed managers.
Introduce a new feature flag for enabling support for multi feeds managers.
graham-chainlink
force-pushed
the
ggoh/OPCORE-863/multiple-feeds-manager-start
branch
2 times, most recently
from
419e5e3
to
94e7f62
Compare
No longer used, since it was replaced by ListJobProposalsByManagersIDs
graham-chainlink
force-pushed
the
ggoh/OPCORE-863/multiple-feeds-manager-start
branch
from
94e7f62
to
5220fb9
Compare
Yes, if no value is set in the config.toml, then the default boolean value kicks in which is false. |
| // | ||
| // When we support multiple feed managers, we will need to change this to filter | ||
| // by feeds manager | ||
| func (s *service) ListJobProposals(ctx context.Context) ([]JobProposal, error) { |
There was a problem hiding this comment.
Who called this method? Shouldn't the call site be replaced with the ListJobProposalsByManagerID? Unless I missed that.
There was a problem hiding this comment.
I documented the reason in my commit description here
We already have ListJobProposalsByManagersIDs which takes in a list of IDs, so we dont need to introduce another ListJobProposalsByManagerID for single ID
|
Reviewers: Most of the files are just test config file changes, PR is not that big.
I also merged this PR into this branch which made this PR bigger (silly mistake), so recommended to review via commits instead.
What
In order to support connection to multiple feed managers, when on start of the node, we want to attempt to connect to list
of registered feed managers.
New feature toggle is introduced.
first commit: actual change to start multiple feeds managers
second commit: hide new behaviour under a new feature flag
Why
In the upcoming CCIP v2, we want to be able to allow it talk to existing nodes that have been registered with CLO.
JIRA: https://smartcontract-it.atlassian.net/browse/OPCORE-863
Note
This is not the only change to enable support for multiple feeds manager.
I have another PR soon which addresses the hardcoding restriction of 1 feeds manager.
UI will need to be tweaked to show multiple feed managers as well.