LOOPP plugin config validation service #12430
Conversation
george-dorin
commented
Mar 14, 2024
•
george-dorin
commented
- Instantiate and call validation when a generic plugin config is validated
|
I see that you haven't updated any README files. Would it make sense to do so? |
…-validation # Conflicts: # core/scripts/go.mod # core/scripts/go.sum # go.mod # go.sum # integration-tests/go.mod # integration-tests/go.sum # integration-tests/load/go.mod # integration-tests/load/go.sum
|
I see you updated files related to |
| } | ||
|
|
||
| loopID := fmt.Sprintf("%s-%s-%s", p.PluginName, spec.ContractID, spec.GetID()) | ||
| cmdFn, grpcOpts, err := rc.RegisterLOOP(plugins.CmdConfig{ |
There was a problem hiding this comment.
It seems a bit odd that we're registering the LOOPP only to immediately unregister it. Do we even need to register it, and if so, why?
There was a problem hiding this comment.
When the plugin is started it requires a prometheusPort, in order to get a valid port we must register the LOOPP
There was a problem hiding this comment.
hmm, this seems inverted.
what is the goal of this code?
another red flag is starting and closing the plugin. why is that happening?
if we register and unregister for the purpose of getting a port and validating a port, then we are still open to failure later when re-register it because there is dynamic code to allocate the port. moreover, the port will be different, so what use is the first registration?
There was a problem hiding this comment.
@krehermann To validate the config we must start the binary so we can call the validateConfig() that is inside the LOOPP, this has to happen inside the LOOPP because we do not know what configuration it expects. When the LOOPP is started, as part of the registration, a Prometheus port is expected, that is why we are registering the LOOPP.
There was a problem hiding this comment.
@krehermann We thought about this quite a bit, and @george-dorin is correct -- I agree that it's counter-intuitive to register and deregister immediately but it's not clear what alternative there is given that the point of registering is to "lock" host resources such as the prometheus port.
There was a problem hiding this comment.
The point of this code is just to call out the LOOPP to validate the config; we aren't trying to do any validation with respect to allocation of system resources and we aren't guaranteeing that a successful validation implies that we will definitely start a LOOPP either.
…-validation # Conflicts: # core/scripts/go.mod # core/scripts/go.sum # go.mod # go.sum # integration-tests/go.mod # integration-tests/go.sum # integration-tests/load/go.mod # integration-tests/load/go.sum
…-validation # Conflicts: # core/scripts/go.mod # core/scripts/go.sum # go.mod # go.sum # integration-tests/go.mod # integration-tests/go.sum # integration-tests/load/go.mod # integration-tests/load/go.sum
…-validation # Conflicts: # core/scripts/go.mod # core/scripts/go.sum # go.mod # go.sum # integration-tests/go.mod # integration-tests/go.sum # integration-tests/load/go.mod # integration-tests/load/go.sum
…-validation # Conflicts: # integration-tests/go.mod # integration-tests/load/go.mod
|
george-dorin
deleted the
chore/BCF-2777-reporting-plugin-LOOPP-job-validation
branch
…ersion * develop: (32 commits) [KS-136] Write target fixes (#12743) chore/release 2.10.0 to develop (#12740) [KS-136] Disallow non-trigger steps with no dependent ref (#12742) [KS-136] Correctly handle numbers in YAML by converting them to floats or ints (#12739) New log buffer (#12357) [KS-101] Add OCR3 capability contract wrapper (#12404) core/services/relay/evm: switch RequestRound DB & Tracker to use sqlutil.DataSource (#12706) Unregister filters for old coordinator contracts contract addresses from Functions LogPollerWrapper (#12696) Add table support for capability "type" property (#12622) Backout CRIB setup on develop. (#12705) fix node upgrade test (#12702) Reduces changeset scope to `minor` for semver (#12699) rm oz dep (#12700) @chainlink.contracts release v1.0.0 (#11714) feat: contracts publishing in CI (#12102) Bump default PG conns from 20->100; enable auto-scaling open conns for mercury (#12697) chore: chainlink-github-actions/* to v2.3.10 (#12694) LOOPP plugin config validation service (#12430) [TT-924] Migrate functions load tests to Seth (#12659) Enhance automation test config (AUTO-9430) (#12689) ...
