Skip to content
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

Merged
merged 2 commits into from
Jun 1, 2021

Conversation

Qinusty
Copy link
Contributor

@Qinusty Qinusty commented May 26, 2021

Introduces kafka support for scheduler and simplifies logic away from scheduler/main.go.

Additionally the dependency on github.com/jordan-wright/ossmalware is replaced with github.com/ossf/package-feeds for the Package 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.

@Qinusty Qinusty changed the title Add Kafka support Add Kafka support for scheduler May 26, 2021
Copy link
Contributor

@oliverchang oliverchang left a 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/main.go Outdated Show resolved Hide resolved
msg, err := proxy.subscription.Receive(ctx)
if err != nil {
log.Println("error receiving message: ", err)
continue
Copy link
Contributor

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.

Copy link
Contributor Author

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.

scheduler/proxy/proxy.go Outdated Show resolved Hide resolved
@tom--pollard
Copy link
Contributor

Part of the issue #70

@Qinusty Qinusty requested a review from oliverchang May 28, 2021 09:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants