conduit: pipeline run loop implementation #1183
Conversation
Eric-Warehime
added
Not-Yet-Enabled
Eric-Warehime
requested review from
winder,
shiqizng,
AlgoStephenAkiki and
algoganesh
Codecov Report
@@ Coverage Diff @@
## conduit #1183 +/- ##
==========================================
Coverage ? 59.04%
==========================================
Files ? 64
Lines ? 8890
Branches ? 0
==========================================
Hits ? 5249
Misses ? 3172
Partials ? 469 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
conduit/pipeline.go
Outdated
| exporterLogger := log.New() | ||
| exporterLogger.SetFormatter( | ||
| PluginLogFormatter{ | ||
| Formatter: &log.JSONFormatter{ | ||
| DisableHTMLEscape: true, | ||
| }, | ||
| Type: "Exporter", | ||
| Name: (*p.exporter).Metadata().Name(), | ||
| }, | ||
| ) |
There was a problem hiding this comment.
We should probably have a helper to construct these.
| ) | ||
|
|
||
| jsonEncode := string(json.Encode(p.cfg.Exporter.Config)) | ||
| err := (*p.exporter).Init(plugins.PluginConfig(jsonEncode), exporterLogger) |
There was a problem hiding this comment.
Could you put a comment somewhere that this is imported first so that we can fetch the round?
Would we ever want multiple exporters, which round do we choose in that scenario?
There was a problem hiding this comment.
I'll update w/ comment.
I think it's hard to enumerate all of possible combinations of multiple exporters. I'd prefer to make it really easy to run a pipeline so that people can just spin up multiple pipelines if they need to run multiple exporters.
| } | ||
|
|
||
| func (p *pipelineImpl) Start() error { | ||
| p.logger.Infof("Starting Pipeline Initialization") | ||
|
|
||
| // TODO Need to change interfaces to accept config of map[string]interface{} | ||
|
|
||
| // Initialize Exporter first since the pipeline derives its round from the Exporter | ||
| exporterLogger := log.New() |
There was a problem hiding this comment.
Thinking about this further, I think we might have a concurrency issue here. Created this ticket for us to follow up later: #1187
Summary
I've gone through the initial implementation of the conduit pipeline run loop, and found/fixed a few bugs along the way.
Bug fixes/changes:
I know of a few more bugs that I wasn't able to address here that I will create tickets for.
Test Plan
This PR runs using local algod importer, (noop/block_processor) processors, and (noop/postgresql) exporters.