-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
reload db-credentials-file upon SIGHUP #4514
Conversation
Signed-off-by: Alex Charis <[email protected]>
Signed-off-by: Alex Charis <[email protected]>
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.
Some nits.
go/vt/dbconfigs/credentials.go
Outdated
sigChan := make(chan os.Signal, 1) | ||
signal.Notify(sigChan, syscall.SIGHUP) | ||
go func() { | ||
for { |
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.
Use for _ = range sigChan {
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.
gofmt changed that to for range sigChan
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.
and i made the same change the two other places i saw the bare for
go/vt/dbconfigs/credentials.go
Outdated
go func() { | ||
for { | ||
<-sigChan | ||
fcs, ok := AllCredentialsServers["file"].(*FileCredentialsServer) |
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.
Inline this with if fcs...; ok {
Signed-off-by: Alex Charis <[email protected]>
why we want this is a bit of a long story, but reloading creds on receipt of SIGHUP seems like a good idea.
Signed-off-by: Alex Charis [email protected]