ibzib commented on a change in pull request #13072:
URL: https://github.com/apache/beam/pull/13072#discussion_r503424490



##########
File path: sdks/go/gogradle.lock
##########
@@ -5,7 +5,7 @@ dependencies:
   build:
   - vcs: "git"
     name: "cloud.google.com/go"
-    commit: "4f6c921ec566a33844f4e7879b31cd8575a6982d"
+    commit: "03869a08dc16b35ad4968e92d34c5a2a2961b205"

Review comment:
       This PR shouldn't require any changes to dependencies.

##########
File path: sdks/go/pkg/beam/runners/dataflow/dataflowlib/job.go
##########
@@ -262,3 +272,36 @@ func printOptions(opts *JobOptions, images []string) 
[]*displayData {
        }
        return ret
 }
+
+func validateWorkerSettings(opts *JobOptions) error {

Review comment:
       Add tests for this function.

##########
File path: sdks/go/pkg/beam/runners/dataflow/dataflowlib/job.go
##########
@@ -262,3 +272,36 @@ func printOptions(opts *JobOptions, images []string) 
[]*displayData {
        }
        return ret
 }
+
+func validateWorkerSettings(opts *JobOptions) error {
+       if opts.Zone != "" && opts.WorkerRegion != "" {
+               return errors.New("cannot use option zone with workerRegion; 
prefer either workerZone or workerRegion")
+       }
+       if opts.Zone != "" && opts.WorkerZone != "" {
+               return errors.New("cannot use option zone with workerZone; 
prefer workerZone")
+       }
+       if opts.WorkerZone != "" && opts.WorkerRegion != "" {
+               return errors.New("workerRegion and workerZone options are 
mutually exclusive")
+       }
+
+       hasExperimentWorkerRegion := false
+       for _, experiment := range opts.Experiments {
+               if strings.HasPrefix(experiment, "worker_region") {
+                       hasExperimentWorkerRegion = true
+                       break
+               }
+       }
+
+       if hasExperimentWorkerRegion && opts.WorkerRegion != "" {
+               return errors.New("experiment worker_region and option 
workerRegion are mutually exclusive")
+       }
+       if hasExperimentWorkerRegion && opts.WorkerZone != "" {
+               return errors.New("experiment worker_region and option 
workerZone are mutually exclusive")
+       }
+
+       if opts.Zone != "" {

Review comment:
       Print a warning that zone is deprecated and worker_zone should be used 
instead.

##########
File path: sdks/go/pkg/beam/runners/dataflow/dataflowlib/job.go
##########
@@ -262,3 +272,36 @@ func printOptions(opts *JobOptions, images []string) 
[]*displayData {
        }
        return ret
 }
+
+func validateWorkerSettings(opts *JobOptions) error {
+       if opts.Zone != "" && opts.WorkerRegion != "" {
+               return errors.New("cannot use option zone with workerRegion; 
prefer either workerZone or workerRegion")
+       }
+       if opts.Zone != "" && opts.WorkerZone != "" {
+               return errors.New("cannot use option zone with workerZone; 
prefer workerZone")
+       }
+       if opts.WorkerZone != "" && opts.WorkerRegion != "" {
+               return errors.New("workerRegion and workerZone options are 
mutually exclusive")
+       }
+
+       hasExperimentWorkerRegion := false
+       for _, experiment := range opts.Experiments {
+               if strings.HasPrefix(experiment, "worker_region") {
+                       hasExperimentWorkerRegion = true
+                       break
+               }
+       }
+
+       if hasExperimentWorkerRegion && opts.WorkerRegion != "" {

Review comment:
       There also needs to be a check for `hasExperimentWorkerRegion && 
opts.Zone != ""`.




----------------------------------------------------------------
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.

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


Reply via email to