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

blog: "Writing and debugging integration tests of multiple processes with Golang" contains error #1589

Closed
marco-m opened this issue Jul 4, 2023 · 2 comments
Assignees

Comments

@marco-m
Copy link

marco-m commented Jul 4, 2023

Hello,

thanks for your Go blog! (and also for dolt, although I didn't have the occasion yet to use it).

I had a quick look but could not find the blog source on github/dolthub, so I am opening this issue here.

The blog post https://www.dolthub.com/blog/2023-05-25-debugging-multiple-golang-processes/ contains an example code with an error:

func (s *SqlServer) GracefulStop() error {
	s.Cmd.Process.Kill()                           // <= HERE
	err := s.Cmd.Process.Signal(syscall.SIGTERM)
	if err != nil {
		return err
	}
	<-s.Done
	return s.Cmd.Wait()
}

s.Cmd.Process.Kill() is not graceful, so it should be sent after SIGTERM (and a timeout) or should not be sent at all.

Looking in the dolt code, I found https://github.com/dolthub/dolt/blob/main/go/libraries/doltcore/dtestutils/sql_server_driver/cmd_unix.go#L29, so it seems it is already fixed there. What it missing is to remove the line from the blog article.

@fulghum
Copy link
Contributor

fulghum commented Jul 5, 2023

Hey @marco-m, thanks for reading our Golang blogs and taking the time to report an issue in the sample code.

@zachmu – you wanna take a look at this one and pull that line from the sample?

@zachmu
Copy link
Member

zachmu commented Oct 12, 2023

Good catch! I'll go ahead and remove that from the blog today.

@zachmu zachmu self-assigned this Oct 12, 2023
@timsehn timsehn closed this as completed Jan 30, 2024
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

No branches or pull requests

4 participants