-
Notifications
You must be signed in to change notification settings - Fork 105
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
chore(vald): update tm-events dependency #1326
Conversation
for _, subscription := range subscriptions { | ||
<-subscription.Done() | ||
} |
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 change is independent of the dependency update, but the cleanup should wait for all subscriptions to complete
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.
There was an issue noticed by a validator that vald
would not stop with a SIGTERM signal. It seemed like vald never cleaned up properly? #1016
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'm not sure this will solve this, but it is a step in the right direction :)
@@ -34,119 +34,10 @@ require ( | |||
google.golang.org/protobuf v1.27.1 | |||
) | |||
|
|||
require ( | |||
filippo.io/edwards25519 v1.0.0-beta.2 // indirect |
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 was all unnecessary??
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 think there are some issues in the team with using different golang versions. When 1.18 is released we should all switch to that
for _, subscription := range subscriptions { | ||
<-subscription.Done() | ||
} |
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.
There was an issue noticed by a validator that vald
would not stop with a SIGTERM signal. It seemed like vald never cleaned up properly? #1016
Description
Bring the tm-events dependency up to date. This means addressing a small change in the jobs.Job signature