[GitHub] [beam] damccorm commented on a diff in pull request #22618: fix minor unreachable code caused by log.Fatal

[GitBox <gi...@apache.org>]

damccorm commented on code in PR #22618:
URL: https://github.com/apache/beam/pull/22618#discussion_r943747266


##########
sdks/python/container/boot.go:
##########
@@ -211,7 +211,7 @@ func main() {
 	for _, workerId := range workerIds {
 		go func(workerId string) {
 			log.Printf("Executing: python %v", strings.Join(args, " "))
-			log.Fatalf("Python exited: %v", execx.ExecuteEnv(map[string]string{"WORKER_ID": workerId}, "python", args...))
+			log.Printf("Python exited: %v", execx.ExecuteEnv(map[string]string{"WORKER_ID": workerId}, "python", args...))
 			wg.Done()

Review Comment:
   ```suggestion
   			wg.Done()
   			log.Fatalf("Python exited: %v", execx.ExecuteEnv(map[string]string{"WORKER_ID": workerId}, "python", args...))
   ```
   
   Instead of downgrading this, we should probably just move it behind `wg.Done()`. This does still represent a failure case



##########
sdks/typescript/boot.go:
##########
@@ -169,6 +169,6 @@ func main() {
 		args = append(args, "--status_endpoint="+info.GetStatusEndpoint().GetUrl())
 	}
 
-	log.Fatalf("User program exited: %v", execx.Execute("npx", args...))
+	log.Printf("User program exited: %v", execx.Execute("npx", args...))
 	log.Printf("SDK exited cleanly.")

Review Comment:
   Similarly here, we probably should just remove the Printf statement since if it makes it to the user, this is a failure case.
   ```suggestion
   	log.Fatalf("User program exited: %v", execx.Execute("npx", args...))
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org