Fixes up admin cluster command for adding search attributes to be indexed (#514)

Changed the --search_attr_type flag to take in an English readable name (e.g. Keyword, Int, String, etc) instead of an integer (which is less intuitive and should be hidden from the end user anyway).

Fixes confusion for the user around which integer to map to the type of field they want indexed.
Removes the need to document the attribute type integer in our temporal docs (which just causes confusion)

Author: mastermanu

tools/cli/admin.go

@@ -798,10 +798,9 @@ func newAdminClusterCommands() []cli.Command {
 					Name:  FlagSearchAttributesKey,
 					Usage: "Search Attribute key to be whitelisted",
 				},
-				cli.IntFlag{
+				cli.StringFlag{
 					Name:  FlagSearchAttributesType,
-					Value: -1,
-					Usage: "Search Attribute value type. [0:String, 1:Keyword, 2:Int, 3:Double, 4:Bool, 5:Datetime]",
+					Usage: "Search Attribute value type. [string, keyword, int, double, bool, datetime]",
 				},
 				cli.StringFlag{
 					Name:  FlagSecurityTokenWithAlias,

tools/cli/adminClusterCommands.go

@@ -37,26 +37,32 @@ import (
 // AdminAddSearchAttribute to whitelist search attribute
 func AdminAddSearchAttribute(c *cli.Context) {
 	key := getRequiredOption(c, FlagSearchAttributesKey)
-	valType := getRequiredIntOption(c, FlagSearchAttributesType)
-	if !isValueTypeValid(valType) {
-		ErrorAndExit("Unknown Search Attributes value type.", nil)
+	valTypeStr := getRequiredOption(c, FlagSearchAttributesType)
+	valTypeInt, err := stringToEnum(valTypeStr, enumspb.IndexedValueType_value)
+	if err != nil {
+		ErrorAndExit("Failed to parse Search Attribute Type", err)
 	}
 
 	// ask user for confirmation
-	promptMsg := fmt.Sprintf("Are you trying to add key [%s] with Type [%s]? Y/N", color.YellowString(key), color.YellowString(intValTypeToString(valType)))
+	promptMsg := fmt.Sprintf(
+		"Are you trying to add key [%s] with Type [%s]? Y/N",
+		color.YellowString(key),
+		color.YellowString(enumspb.IndexedValueType_name[valTypeInt]),
+	)
+
 	prompt(promptMsg, c.GlobalBool(FlagAutoConfirm))
 
 	adminClient := cFactory.AdminClient(c)
 	ctx, cancel := newContext(c)
 	defer cancel()
 	request := &adminservice.AddSearchAttributeRequest{
 		SearchAttribute: map[string]enumspb.IndexedValueType{
-			key: enumspb.IndexedValueType(valType),
+			key: enumspb.IndexedValueType(valTypeInt),
 		},
 		SecurityToken: c.String(FlagSecurityToken),
 	}
 
-	_, err := adminClient.AddSearchAttribute(ctx, request)
+	_, err = adminClient.AddSearchAttribute(ctx, request)
 	if err != nil {
 		ErrorAndExit("Add search attribute failed.", err)
 	}
@@ -76,26 +82,3 @@ func AdminDescribeCluster(c *cli.Context) {
 
 	prettyPrintJSONObject(response)
 }
-
-func intValTypeToString(valType int) string {
-	switch valType {
-	case 0:
-		return "String"
-	case 1:
-		return "Keyword"
-	case 2:
-		return "Int"
-	case 3:
-		return "Double"
-	case 4:
-		return "Bool"
-	case 5:
-		return "Datetime"
-	default:
-		return ""
-	}
-}
-
-func isValueTypeValid(valType int) bool {
-	return valType >= 0 && valType <= 5
-}

tools/cli/adminClusterCommands_test.go

@@ -23,42 +23,3 @@
 // THE SOFTWARE.
 
 package cli
-
-import (
-	"testing"
-
-	"github.com/bmizerany/assert"
-)
-
-func TestAdminAddSearchAttribute_isValueTypeValid(t *testing.T) {
-	testCases := []struct {
-		name     string
-		input    int
-		expected bool
-	}{
-		{
-			name:     "negative",
-			input:    -1,
-			expected: false,
-		},
-		{
-			name:     "valid",
-			input:    0,
-			expected: true,
-		},
-		{
-			name:     "valid",
-			input:    5,
-			expected: true,
-		},
-		{
-			name:     "unknown",
-			input:    6,
-			expected: false,
-		},
-	}
-
-	for _, testCase := range testCases {
-		assert.Equal(t, testCase.expected, isValueTypeValid(testCase.input))
-	}
-}

tools/cli/app_test.go

@@ -543,12 +543,12 @@ func (s *cliAppSuite) TestAdminDescribeWorkflow_Failed() {
 func (s *cliAppSuite) TestAdminAddSearchAttribute() {
 	request := &adminservice.AddSearchAttributeRequest{
 		SearchAttribute: map[string]enumspb.IndexedValueType{
-			"testKey": enumspb.IndexedValueType(1),
+			"testKey": enumspb.IndexedValueType(2),
 		},
 	}
 	s.serverAdminClient.EXPECT().AddSearchAttribute(gomock.Any(), request).Times(1)
 
-	err := s.app.Run([]string{"", "--auto_confirm", "--ns", cliTestNamespace, "admin", "cl", "asa", "--search_attr_key", "testKey", "--search_attr_type", "1"})
+	err := s.app.Run([]string{"", "--auto_confirm", "--ns", cliTestNamespace, "admin", "cl", "asa", "--search_attr_key", "testKey", "--search_attr_type", "keyword"})
 	s.Nil(err)
 }