-
Notifications
You must be signed in to change notification settings - Fork 51
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Kafka support for scheduler #77
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks! some minor suggestions.
scheduler/proxy/proxy.go
Outdated
msg, err := proxy.subscription.Receive(ctx) | ||
if err != nil { | ||
log.Println("error receiving message: ", err) | ||
continue |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a problem that existed prior to your PR as well, but if Receive gets an error, it'll never succeed again per: https://pkg.go.dev/gocloud.dev/pubsub#Subscription.Receive
We need to either return here or recreate the subscription before retrying.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've returned here to avoid the application hanging with a broken Subscription, going forwards we should look to reconstruct the Subscription before retrying as you mention.
c506734
to
ed7d819
Compare
Part of the issue #70 |
Introduces kafka support for scheduler and simplifies logic away from scheduler/main.go.
Additionally the dependency on
github.com/jordan-wright/ossmalware
is replaced withgithub.com/ossf/package-feeds
for thePackage
struct definition.The
package-feeds
dependency introduces the updated struct in relation to #48.Additional kafka support will need to be added for the analysis workers. Also we probably want to consider the range implementations of pubsub supported by gocloud as these will need to be imported.