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


##########
sdks/go/pkg/beam/runners/dataflow/dataflowlib/job.go:
##########
@@ -240,29 +240,46 @@ func WaitForCompletion(ctx context.Context, client 
*df.Service, project, region,
                        return errors.Wrap(err, "failed to get job")
                }
 
-               switch j.CurrentState {
-               case "JOB_STATE_DONE":
-                       log.Info(ctx, "Job succeeded!")
-                       return nil
-
-               case "JOB_STATE_CANCELLED":
-                       log.Info(ctx, "Job cancelled")
+               terminal, msg, err := currentStateMessage(j.CurrentState, jobID)
+               if err != nil {
+                       return err
+               }
+               log.Infof(ctx, msg)
+               if terminal {
                        return nil
-
-               case "JOB_STATE_FAILED":
-                       return errors.Errorf("job %s failed", jobID)
-
-               case "JOB_STATE_RUNNING":
-                       log.Info(ctx, "Job still running ...")
-
-               default:
-                       log.Infof(ctx, "Job state: %v ...", j.CurrentState)
                }
 
                time.Sleep(30 * time.Second)
        }
 }
 
+// currentStateMessage indicates if the state is terminal, and provides a 
message to log, or an error.
+// Errors are always terminal.
+func currentStateMessage(currentState, jobID string) (bool, string, error) {
+       switch currentState {
+       // Add all Terminal Success stats here.
+       case "JOB_STATE_DONE", "JOB_STATE_CANCELLED", "JOB_STATE_DRAINED", 
"JOB_STATE_UPDATED":
+               var state string
+               switch currentState {
+               case "JOB_STATE_DONE":
+                       state = "succeeded"

Review Comment:
   ```suggestion
                        state = "succeeded!"
   ```
   
   Totally optional (and it would require a small test tweak too), but I liked 
the exclamation point! It brought me some small joy/excitement haha



##########
sdks/go/pkg/beam/runners/dataflow/dataflowlib/job_test.go:
##########
@@ -157,3 +158,39 @@ func TestValidateWorkerSettings(t *testing.T) {
                })
        }
 }
+
+func TestCurrentStateMessage(t *testing.T) {
+       tests := []struct {
+               state   string
+               term    bool
+               want    string
+               wantErr error
+       }{
+               {state: "JOB_STATE_DONE", want: "Job JorbID-09876 succeeded", 
term: true},

Review Comment:
   ```suggestion
                {state: "JOB_STATE_DONE", want: "Job JorbID-09876 succeeded!", 
term: true},
   ```
   
   Putting this here in case you want to quick commit both 😉 



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to