Last bits of review comments

temporalio · May 25, 2023 · c6734d1 · c6734d1
1 parent f3d24e2
commit c6734d1
Show file tree

Hide file tree

Showing 7 changed files with 69 additions and 14 deletions.
diff --git a/internal/activity.go b/internal/activity.go
@@ -148,6 +148,7 @@ type (
 
 		// VersioningIntent specifies whether this activity should run on a worker with a compatible
 		// build ID or not. See temporal.VersioningIntent.
+		// WARNING: Worker versioning is currently experimental
 		VersioningIntent VersioningIntent
 	}
 

diff --git a/internal/worker_version_sets.go b/internal/worker_version_sets.go
@@ -42,9 +42,9 @@ const (
 	// version if possible. It may not be possible if the target task queue does not also have
 	// knowledge of the current worker's build ID.
 	VersioningIntentCompatible
-	// VersioningIntentUseDefault indicates that the command should run on the target task queue's
+	// VersioningIntentDefault indicates that the command should run on the target task queue's
 	// current overall-default build ID.
-	VersioningIntentUseDefault
+	VersioningIntentDefault
 )
 
 type (
@@ -185,7 +185,7 @@ func (v *BuildIDOpPromoteIDWithinSet) targetedBuildId() string      { return v.B
 // the user's intent and whether the target task queue matches this worker's task queue.
 func determineUseCompatibleFlagForCommand(intent VersioningIntent, workerTq, TargetTq string) bool {
 	useCompat := true
-	if intent == VersioningIntentUseDefault {
+	if intent == VersioningIntentDefault {
 		useCompat = false
 	} else if intent == VersioningIntentUnspecified {
 		// If the target task queue doesn't match ours, use the default version

diff --git a/internal/worker_version_sets_test.go b/internal/worker_version_sets_test.go
@@ -63,3 +63,60 @@ func Test_WorkerVersionSets_fromProtoResponse(t *testing.T) {
 		})
 	}
 }
+
+func Test_VersioningIntent(t *testing.T) {
+	tests := []struct {
+		name                string
+		intent              VersioningIntent
+		tqSame              bool
+		shouldUseCompatible bool
+	}{
+		{
+			name:                "Unspecified same TQ",
+			intent:              VersioningIntentUnspecified,
+			tqSame:              true,
+			shouldUseCompatible: true,
+		},
+		{
+			name:                "Unspecified different TQ",
+			intent:              VersioningIntentUnspecified,
+			tqSame:              false,
+			shouldUseCompatible: false,
+		},
+		{
+			name:                "Default same TQ",
+			intent:              VersioningIntentDefault,
+			tqSame:              true,
+			shouldUseCompatible: false,
+		},
+		{
+			name:                "Default different TQ",
+			intent:              VersioningIntentDefault,
+			tqSame:              false,
+			shouldUseCompatible: false,
+		},
+		{
+			name:                "Compatible same TQ",
+			intent:              VersioningIntentCompatible,
+			tqSame:              true,
+			shouldUseCompatible: true,
+		},
+		{
+			name:                "Compatible different TQ",
+			intent:              VersioningIntentCompatible,
+			tqSame:              false,
+			shouldUseCompatible: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			tqA := "a"
+			tqB := "b"
+			if tt.tqSame {
+				tqB = tqA
+			}
+			assert.Equal(t,
+				tt.shouldUseCompatible, determineUseCompatibleFlagForCommand(tt.intent, tqA, tqB))
+		})
+	}
+}
diff --git a/internal/workflow.go b/internal/workflow.go
@@ -300,6 +300,7 @@ type (
 
 		// VersioningIntent specifies whether this child workflow should run on a worker with a
 		// compatible build ID or not. See VersioningIntent.
+		// WARNING: Worker versioning is currently experimental
 		VersioningIntent VersioningIntent
 	}
 
@@ -1388,6 +1389,7 @@ func WithDataConverter(ctx Context, dc converter.DataConverter) Context {
 
 // WithWorkflowVersioningIntent is used to set the VersioningIntent before constructing a
 // ContinueAsNewError with NewContinueAsNewError.
+// WARNING: Worker versioning is currently experimental
 func WithWorkflowVersioningIntent(ctx Context, intent VersioningIntent) Context {
 	ctx1 := setWorkflowEnvOptionsIfNotExist(ctx)
 	getWorkflowEnvOptions(ctx1).VersioningIntent = intent

diff --git a/internal/workflow_test.go b/internal/workflow_test.go
@@ -56,8 +56,7 @@ func TestGetChildWorkflowOptions(t *testing.T) {
 			"foo": "bar",
 		},
 		ParentClosePolicy: enums.PARENT_CLOSE_POLICY_REQUEST_CANCEL,
-		// TODO(bergundy): test the defaults here with caller and destination same task queue / different task queue
-		VersioningIntent: VersioningIntentUseDefault,
+		VersioningIntent:  VersioningIntentDefault,
 	}
 
 	// Require test options to have non-zero value for each field. This ensures that we update tests (and the
@@ -78,8 +77,7 @@ func TestGetActivityOptions(t *testing.T) {
 		ActivityID:             "bar",
 		RetryPolicy:            newTestRetryPolicy(),
 		DisableEagerExecution:  true,
-		// TODO(bergundy): test the defaults here with caller and destination same task queue / different task queue
-		VersioningIntent: VersioningIntentUseDefault,
+		VersioningIntent:       VersioningIntentDefault,
 	}
 
 	assertNonZero(t, opts)

diff --git a/temporal/build_id_versioning.go b/temporal/build_id_versioning.go
@@ -40,8 +40,8 @@ const (
 	// knowledge of the current worker's build ID.
 	// WARNING: Worker versioning is currently experimental
 	VersioningIntentCompatible = internal.VersioningIntentCompatible
-	// VersioningIntentUseDefault indicates that the command should run on the target task queue's
+	// VersioningIntentDefault indicates that the command should run on the target task queue's
 	// current overall-default build ID.
 	// WARNING: Worker versioning is currently experimental
-	VersioningIntentUseDefault = internal.VersioningIntentUseDefault
+	VersioningIntentDefault = internal.VersioningIntentDefault
 )
diff --git a/test/go.sum b/test/go.sum
@@ -913,7 +913,6 @@ github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/
 github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
 github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU=
 github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4=
-github.com/stretchr/testify v1.8.2/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4=
 github.com/stretchr/testify v1.8.3 h1:RP3t2pwF7cMEbC1dqtB6poj3niw/9gnV4Cjg5oW5gtY=
 github.com/stretchr/testify v1.8.3/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo=
 github.com/twmb/murmur3 v1.1.5 h1:i9OLS9fkuLzBXjt6dptlAEyk58fJsSTXbRg3SgVyqgk=
@@ -946,8 +945,6 @@ go.opentelemetry.io/otel/trace v1.2.0/go.mod h1:N5FLswTubnxKxOJHM7XZC074qpeEdLy3
 go.opentelemetry.io/proto/otlp v0.7.0/go.mod h1:PqfVotwruBrMGOCsRd/89rSnXhoiJIqeYNgFYFoEGnI=
 go.opentelemetry.io/proto/otlp v0.15.0/go.mod h1:H7XAot3MsfNsj7EXtrA2q5xSNQ10UqI405h3+duxN4U=
 go.opentelemetry.io/proto/otlp v0.19.0/go.mod h1:H7XAot3MsfNsj7EXtrA2q5xSNQ10UqI405h3+duxN4U=
-go.temporal.io/api v1.19.1-0.20230524162623-0e1dbb54f8e4 h1:jPO/S2h2StE9OTvXVj7lKIQ5NA7KBHhoU+EOcMQZniE=
-go.temporal.io/api v1.19.1-0.20230524162623-0e1dbb54f8e4/go.mod h1:HHNMo8QTfbYTTAbJ758tBhUNY0Bm/Mxp1EQeEjZhrFQ=
 go.temporal.io/api v1.19.1-0.20230525203837-c5e9155968d8 h1:5pjw7zfZgvKSfjU+eEpk9aRzov+Ho3jwiEOxQz0eork=
 go.temporal.io/api v1.19.1-0.20230525203837-c5e9155968d8/go.mod h1:xlsUEakkN2vU2/WV7e5NqMG4N93nfuNfvbXdaXUpU8w=
 go.uber.org/atomic v1.4.0/go.mod h1:gD2HeocX3+yG+ygLZcrzQJaqmWj9AIm7n08wl/qW/PE=
@@ -1525,8 +1522,8 @@ google.golang.org/genproto v0.0.0-20230320184635-7606e756e683/go.mod h1:NWraEVix
 google.golang.org/genproto v0.0.0-20230323212658-478b75c54725/go.mod h1:UUQDJDOlWu4KYeJZffbWgBkS1YFobzKbLVfK69pe0Ak=
 google.golang.org/genproto v0.0.0-20230330154414-c0448cd141ea/go.mod h1:UUQDJDOlWu4KYeJZffbWgBkS1YFobzKbLVfK69pe0Ak=
 google.golang.org/genproto v0.0.0-20230331144136-dcfb400f0633/go.mod h1:UUQDJDOlWu4KYeJZffbWgBkS1YFobzKbLVfK69pe0Ak=
-google.golang.org/genproto v0.0.0-20230410155749-daa745c078e1 h1:KpwkzHKEF7B9Zxg18WzOa7djJ+Ha5DzthMyZYQfEn2A=
-google.golang.org/genproto v0.0.0-20230410155749-daa745c078e1/go.mod h1:nKE/iIaLqn2bQwXBg8f1g2Ylh6r5MN5CmZvuzZCgsCU=
+google.golang.org/genproto v0.0.0-20230525154841-bd750badd5c6 h1:62QuyPXKEkZpjZesyj5K5jABl6MnSnWl+vNuT5oz90E=
+google.golang.org/genproto v0.0.0-20230525154841-bd750badd5c6/go.mod h1:nKE/iIaLqn2bQwXBg8f1g2Ylh6r5MN5CmZvuzZCgsCU=
 google.golang.org/grpc v1.12.0/go.mod h1:yo6s7OP7yaDglbqo1J04qKzAhqBH6lvTonzMVmEdcZw=
 google.golang.org/grpc v1.19.0/go.mod h1:mqu4LbDTu4XGKhr4mRzUsmM4RtVoemTSY81AxZiDr8c=
 google.golang.org/grpc v1.20.1/go.mod h1:10oTOabMzJvdu6/UiuZezV6QK5dSlG84ov/aaiqXj38=