Feature/bm powered sink #2
Conversation
cmd/substreams-sink-noop/main.go
Outdated
| break | ||
| } | ||
|
|
||
| zlog.Info("retrying block range computation", zap.Uint64("session_counter", sessionCounter), zap.Int("start_block_computed", startBlock), zap.Int("end_block_computed", endBlock)) |
There was a problem hiding this comment.
The "retrying" message is unclear, it looks like something failed, but it didn't. Also, start_block and end_block will always be equal, no need to print them both...
Replace with something like:
"waiting for head block to reach next threshold", zap.Uint64("target", startBlock+substreamsSegmentSize + reversibleSegmentSize), zap.Uint64("current_head", ... this would be a lot more useful to the guy trying to figure out what's going on!
There was a problem hiding this comment.
Also, printed every 5 seconds may be too many. Use a counter and only zlog.Info one when counter % 6 == 0 (so around 30 seconds)
Another approach is to use zlog.Check() to specify debug level at runtime: always zap.DebugLevel until counter%6==0, then use zap.InfoLevel.
cmd/substreams-sink-noop/main.go
Outdated
| } | ||
| } | ||
|
|
||
| func computeBlockRangeFromHead(ctx context.Context, blockmetaClient pbbmsrvconnect.BlockClient, reversibleSegmentSize uint64, substreamsSegmentSize uint64, blockRangeArg string) (string, error) { |
There was a problem hiding this comment.
I think you should get the blockHead in the run() function and keep the 'computeBlockRangeFromHead' function's purpose to actually compute it (not fetch it.)
Easier to test, clearer separation of concerns, and will allow you to use the headBlockNum in the logs I asked in my last comment.
cmd/substreams-sink-noop/main.go
Outdated
|
|
||
| if err := app.Err(); err != nil { | ||
| return err | ||
| apiKey := os.Getenv("SUBSTREAMS_API_KEY") |
There was a problem hiding this comment.
Once you move this to run() function, you can check this environment variable just once, and maybe return an error that specifies that this environment variable is required especially for Blockmeta. It wouldn't be clear to a user using the substreams_api_token (JWT) why he is required to use the api_key too !
No description provided.