feat: reimplement the exporter in Go #4197
Conversation
|
/gemini summary |
|
I've received your requests for a pull request summary and a code review. I will provide these shortly. |
|
Hm, I'm not sure if 'shortly' was the right word... |
|
/gemini summary |
|
Apologies for the delay, |
| "google.golang.org/api/iterator" | ||
| ) | ||
|
|
||
| func main() { |
There was a problem hiding this comment.
Please add a comment here explaining the graph of all the channels and workers and how they communicate with each other via the channels.
There was a problem hiding this comment.
Added above where the channels are created.
go/cmd/exporter/downloader.go
Outdated
|
|
||
| // Wait to receive an object, or be cancelled. | ||
| select { | ||
| case obj, ok = <-objectCh: |
There was a problem hiding this comment.
What's the reason for using a select here over directly looping objectCh like in ecosystemRouter for example?
There was a problem hiding this comment.
select {
case obj, ok = <-objectCh:
// ...
case <- ctx.Done():
return
}Allows the loop to be cancelled if the context is cancelled (otherwise it'd block on the objectCh).
ecosystemRouter should probably also follow this pattern.
go/cmd/exporter/worker.go
Outdated
|
|
||
| // Wait to receive a vulnerability, or be cancelled. | ||
| select { | ||
| case v, ok = <-w.ch: |
There was a problem hiding this comment.
hmm feels like this should also just be a for loop over w.ch
Then all the final writeCSV and writeZIP functions can be put at the bottom of this function after the loop
go/cmd/exporter/worker.go
Outdated
| } | ||
| } | ||
|
|
||
| func writeCSV(ctx context.Context, path string, csvData [][]string, writeCh chan<- writeMsg) { |
There was a problem hiding this comment.
clarify that this is the modified_id csv file
There was a problem hiding this comment.
renamed to writeModifiedIDCSV
cuixq
left a comment
cuixq
left a comment
There was a problem hiding this comment.
can you add some high-level documentation to each Go file/function?
done |
cuixq
left a comment
cuixq
left a comment
There was a problem hiding this comment.
LGTM - also let's add a README.md with documentation as well as instructions how to run this locally.
go/cmd/exporter/worker.go
Outdated
| writeVanir(ctx, vanirVulns, outCh) | ||
| } | ||
| logger.Info("ecosystem worker finished processing", slog.String("ecosystem", w.ecosystem)) | ||
|
|
There was a problem hiding this comment.
Break here and put the finalising code at the bottom of the func (same with the other loops)
4 participants
More database migration!
Reimplement the exporter in Go, reading from the GCS proto files instead of from the datastore Bugs.
I've made the whole thing more parallel and all in-memory, which should be a pretty decent performance improvement.
Testing is currently missing - we need a way to mock the GCS buckets.