From eaeb998c618eda9ed5dce990df35b35bddbf38d6 Mon Sep 17 00:00:00 2001 From: Sean Lin Date: Wed, 4 Aug 2021 20:31:51 -0700 Subject: [PATCH] Render descriptions in flytectl (#154) --- flytectl/cmd/get/execution_util.go | 65 ++++++++++---- flytectl/cmd/get/launch_plan.go | 31 ++++++- flytectl/cmd/get/launch_plan_test.go | 121 ++++++++++++++++++++------- flytectl/cmd/get/task.go | 25 +++++- flytectl/cmd/get/task_test.go | 59 ++++++++++--- flytectl/cmd/get/workflow.go | 23 +++++ flytectl/cmd/testutils/test_utils.go | 9 +- flytectl/go.mod | 1 + flytectl/pkg/printer/printer.go | 65 +++++++++++++- flytectl/pkg/printer/printer_test.go | 54 ++++++++++++ 10 files changed, 384 insertions(+), 69 deletions(-) diff --git a/flytectl/cmd/get/execution_util.go b/flytectl/cmd/get/execution_util.go index 2d70450af3..65b72175a5 100644 --- a/flytectl/cmd/get/execution_util.go +++ b/flytectl/cmd/get/execution_util.go @@ -6,25 +6,25 @@ import ( "io/ioutil" "os" + "gopkg.in/yaml.v3" + cmdUtil "github.com/flyteorg/flytectl/pkg/commandutils" "github.com/flyteorg/flyteidl/clients/go/coreutils" "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin" "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/core" - - "sigs.k8s.io/yaml" ) -// ExecutionConfig is duplicated struct from create with the same structure. This is to avoid the circular dependency. +// ExecutionConfig is duplicated struct from create with the same structure. This is to avoid the circular dependency. Only works with go-yaml. // TODO : replace this with a cleaner design type ExecutionConfig struct { - TargetDomain string `json:"targetDomain"` - TargetProject string `json:"targetProject"` - KubeServiceAcct string `json:"kubeServiceAcct"` - IamRoleARN string `json:"iamRoleARN"` - Workflow string `json:"workflow,omitempty"` - Task string `json:"task,omitempty"` - Version string `json:"version"` - Inputs map[string]interface{} `json:"inputs"` + IamRoleARN string `yaml:"iamRoleARN"` + Inputs map[string]yaml.Node `yaml:"inputs"` + KubeServiceAcct string `yaml:"kubeServiceAcct"` + TargetDomain string `yaml:"targetDomain"` + TargetProject string `yaml:"targetProject"` + Task string `yaml:"task,omitempty"` + Version string `yaml:"version"` + Workflow string `yaml:"workflow,omitempty"` } func WriteExecConfigToFile(executionConfig ExecutionConfig, fileName string) error { @@ -78,16 +78,27 @@ func TaskInputs(task *admin.Task) map[string]*core.Variable { return task.Closure.CompiledTask.Template.Interface.Inputs.Variables } -func ParamMapForTask(task *admin.Task) (map[string]interface{}, error) { +func ParamMapForTask(task *admin.Task) (map[string]yaml.Node, error) { taskInputs := TaskInputs(task) - paramMap := make(map[string]interface{}, len(taskInputs)) + paramMap := make(map[string]yaml.Node, len(taskInputs)) for k, v := range taskInputs { varTypeValue, err := coreutils.MakeDefaultLiteralForType(v.Type) if err != nil { fmt.Println("error creating default value for literal type ", v.Type) return nil, err } - if paramMap[k], err = coreutils.ExtractFromLiteral(varTypeValue); err != nil { + var nativeLiteral interface{} + if nativeLiteral, err = coreutils.ExtractFromLiteral(varTypeValue); err != nil { + return nil, err + } + + if k == v.Description { + // a: # a isn't very helpful + paramMap[k], err = getCommentedYamlNode(nativeLiteral, "") + } else { + paramMap[k], err = getCommentedYamlNode(nativeLiteral, v.Description) + } + if err != nil { return nil, err } } @@ -105,24 +116,42 @@ func WorkflowParams(lp *admin.LaunchPlan) map[string]*core.Parameter { return lp.Spec.DefaultInputs.Parameters } -func ParamMapForWorkflow(lp *admin.LaunchPlan) (map[string]interface{}, error) { +func ParamMapForWorkflow(lp *admin.LaunchPlan) (map[string]yaml.Node, error) { workflowParams := WorkflowParams(lp) - paramMap := make(map[string]interface{}, len(workflowParams)) + paramMap := make(map[string]yaml.Node, len(workflowParams)) for k, v := range workflowParams { varTypeValue, err := coreutils.MakeDefaultLiteralForType(v.Var.Type) if err != nil { fmt.Println("error creating default value for literal type ", v.Var.Type) return nil, err } - if paramMap[k], err = coreutils.ExtractFromLiteral(varTypeValue); err != nil { + var nativeLiteral interface{} + if nativeLiteral, err = coreutils.ExtractFromLiteral(varTypeValue); err != nil { return nil, err } // Override if there is a default value if paramsDefault, ok := v.Behavior.(*core.Parameter_Default); ok { - if paramMap[k], err = coreutils.ExtractFromLiteral(paramsDefault.Default); err != nil { + if nativeLiteral, err = coreutils.ExtractFromLiteral(paramsDefault.Default); err != nil { return nil, err } } + if k == v.Var.Description { + // a: # a isn't very helpful + paramMap[k], err = getCommentedYamlNode(nativeLiteral, "") + } else { + paramMap[k], err = getCommentedYamlNode(nativeLiteral, v.Var.Description) + } + + if err != nil { + return nil, err + } } return paramMap, nil } + +func getCommentedYamlNode(input interface{}, comment string) (yaml.Node, error) { + var node yaml.Node + err := node.Encode(input) + node.LineComment = comment + return node, err +} diff --git a/flytectl/cmd/get/launch_plan.go b/flytectl/cmd/get/launch_plan.go index ec90a62fd2..34bf10cf1a 100644 --- a/flytectl/cmd/get/launch_plan.go +++ b/flytectl/cmd/get/launch_plan.go @@ -104,6 +104,8 @@ var launchplanColumns = []printer.Column{ {Header: "Type", JSONPath: "$.closure.compiledTask.template.type"}, {Header: "State", JSONPath: "$.spec.state"}, {Header: "Schedule", JSONPath: "$.spec.entityMetadata.schedule"}, + {Header: "Inputs", JSONPath: "$.closure.expectedInputs.parameters." + printer.DefaultFormattedDescriptionsKey + ".var.description"}, + {Header: "Outputs", JSONPath: "$.closure.expectedOutputs.variables." + printer.DefaultFormattedDescriptionsKey + ".description"}, } // Column structure for get all launchplans @@ -122,6 +124,21 @@ func LaunchplanToProtoMessages(l []*admin.LaunchPlan) []proto.Message { return messages } +func LaunchplanToTableProtoMessages(l []*admin.LaunchPlan) []proto.Message { + messages := make([]proto.Message, 0, len(l)) + for _, m := range l { + m := proto.Clone(m).(*admin.LaunchPlan) + if m.Closure.ExpectedInputs != nil { + printer.FormatParameterDescriptions(m.Closure.ExpectedInputs.Parameters) + } + if m.Closure.ExpectedOutputs != nil { + printer.FormatVariableDescriptions(m.Closure.ExpectedOutputs.Variables) + } + messages = append(messages, m) + } + return messages +} + func getLaunchPlanFunc(ctx context.Context, args []string, cmdCtx cmdCore.CommandContext) error { launchPlanPrinter := printer.Printer{} var launchPlans []*admin.LaunchPlan @@ -134,8 +151,13 @@ func getLaunchPlanFunc(ctx context.Context, args []string, cmdCtx cmdCore.Comman return err } logger.Debugf(ctx, "Retrieved %v launch plans", len(launchPlans)) - err = launchPlanPrinter.Print(config.GetConfig().MustOutputFormat(), launchplanColumns, - LaunchplanToProtoMessages(launchPlans)...) + if config.GetConfig().MustOutputFormat() == printer.OutputFormatTABLE { + err = launchPlanPrinter.Print(config.GetConfig().MustOutputFormat(), launchplanColumns, + LaunchplanToTableProtoMessages(launchPlans)...) + } else { + err = launchPlanPrinter.Print(config.GetConfig().MustOutputFormat(), launchplanColumns, + LaunchplanToProtoMessages(launchPlans)...) + } if err != nil { return err } @@ -148,8 +170,13 @@ func getLaunchPlanFunc(ctx context.Context, args []string, cmdCtx cmdCore.Comman } logger.Debugf(ctx, "Retrieved %v launch plans", len(launchPlans)) + if config.GetConfig().MustOutputFormat() == printer.OutputFormatTABLE { + return launchPlanPrinter.Print(config.GetConfig().MustOutputFormat(), launchplansColumns, + LaunchplanToTableProtoMessages(launchPlans)...) + } return launchPlanPrinter.Print(config.GetConfig().MustOutputFormat(), launchplansColumns, LaunchplanToProtoMessages(launchPlans)...) + } // FetchLPForName fetches the launchplan give it name. diff --git a/flytectl/cmd/get/launch_plan_test.go b/flytectl/cmd/get/launch_plan_test.go index 9757a35c1d..6e3e104390 100644 --- a/flytectl/cmd/get/launch_plan_test.go +++ b/flytectl/cmd/get/launch_plan_test.go @@ -7,6 +7,7 @@ import ( "github.com/flyteorg/flytectl/pkg/filters" + "github.com/flyteorg/flytectl/cmd/config" "github.com/flyteorg/flytectl/cmd/config/subcommand/launchplan" cmdCore "github.com/flyteorg/flytectl/cmd/core" u "github.com/flyteorg/flytectl/cmd/testutils" @@ -48,6 +49,7 @@ func getLaunchPlanSetup() { }, }, }, + Description: "short desc", }, }, "numbers_count": { @@ -57,6 +59,7 @@ func getLaunchPlanSetup() { Simple: core.SimpleType_INTEGER, }, }, + Description: "long description will be truncated in table", }, }, "run_local_at_count": { @@ -66,6 +69,7 @@ func getLaunchPlanSetup() { Simple: core.SimpleType_INTEGER, }, }, + Description: "run_local_at_count", }, Behavior: &core.Parameter_Default{ Default: &core.Literal{ @@ -258,21 +262,24 @@ func TestGetLaunchPlanFunc(t *testing.T) { "collectionType": { "simple": "INTEGER" } - } + }, + "description": "short desc" } }, "numbers_count": { "var": { "type": { "simple": "INTEGER" - } + }, + "description": "long description will be truncated in table" } }, "run_local_at_count": { "var": { "type": { "simple": "INTEGER" - } + }, + "description": "run_local_at_count" }, "default": { "scalar": { @@ -294,21 +301,24 @@ func TestGetLaunchPlanFunc(t *testing.T) { "collectionType": { "simple": "INTEGER" } - } + }, + "description": "short desc" } }, "numbers_count": { "var": { "type": { "simple": "INTEGER" - } + }, + "description": "long description will be truncated in table" } }, "run_local_at_count": { "var": { "type": { "simple": "INTEGER" - } + }, + "description": "run_local_at_count" }, "default": { "scalar": { @@ -337,21 +347,24 @@ func TestGetLaunchPlanFunc(t *testing.T) { "collectionType": { "simple": "INTEGER" } - } + }, + "description": "short desc" } }, "numbers_count": { "var": { "type": { "simple": "INTEGER" - } + }, + "description": "long description will be truncated in table" } }, "run_local_at_count": { "var": { "type": { "simple": "INTEGER" - } + }, + "description": "run_local_at_count" }, "default": { "scalar": { @@ -373,21 +386,24 @@ func TestGetLaunchPlanFunc(t *testing.T) { "collectionType": { "simple": "INTEGER" } - } + }, + "description": "short desc" } }, "numbers_count": { "var": { "type": { "simple": "INTEGER" - } + }, + "description": "long description will be truncated in table" } }, "run_local_at_count": { "var": { "type": { "simple": "INTEGER" - } + }, + "description": "run_local_at_count" }, "default": { "scalar": { @@ -429,21 +445,24 @@ func TestGetLaunchPlanFuncLatest(t *testing.T) { "collectionType": { "simple": "INTEGER" } - } + }, + "description": "short desc" } }, "numbers_count": { "var": { "type": { "simple": "INTEGER" - } + }, + "description": "long description will be truncated in table" } }, "run_local_at_count": { "var": { "type": { "simple": "INTEGER" - } + }, + "description": "run_local_at_count" }, "default": { "scalar": { @@ -465,21 +484,24 @@ func TestGetLaunchPlanFuncLatest(t *testing.T) { "collectionType": { "simple": "INTEGER" } - } + }, + "description": "short desc" } }, "numbers_count": { "var": { "type": { "simple": "INTEGER" - } + }, + "description": "long description will be truncated in table" } }, "run_local_at_count": { "var": { "type": { "simple": "INTEGER" - } + }, + "description": "run_local_at_count" }, "default": { "scalar": { @@ -520,21 +542,24 @@ func TestGetLaunchPlanWithVersion(t *testing.T) { "collectionType": { "simple": "INTEGER" } - } + }, + "description": "short desc" } }, "numbers_count": { "var": { "type": { "simple": "INTEGER" - } + }, + "description": "long description will be truncated in table" } }, "run_local_at_count": { "var": { "type": { "simple": "INTEGER" - } + }, + "description": "run_local_at_count" }, "default": { "scalar": { @@ -556,21 +581,24 @@ func TestGetLaunchPlanWithVersion(t *testing.T) { "collectionType": { "simple": "INTEGER" } - } + }, + "description": "short desc" } }, "numbers_count": { "var": { "type": { "simple": "INTEGER" - } + }, + "description": "long description will be truncated in table" } }, "run_local_at_count": { "var": { "type": { "simple": "INTEGER" - } + }, + "description": "run_local_at_count" }, "default": { "scalar": { @@ -595,7 +623,7 @@ func TestGetLaunchPlans(t *testing.T) { argsLp = []string{} err = getLaunchPlanFunc(ctx, argsLp, cmdCtx) assert.Nil(t, err) - tearDownAndVerify(t, `[{"id": {"name": "launchplan1","version": "v2"},"spec": {"defaultInputs": {"parameters": {"numbers": {"var": {"type": {"collectionType": {"simple": "INTEGER"}}}},"numbers_count": {"var": {"type": {"simple": "INTEGER"}}},"run_local_at_count": {"var": {"type": {"simple": "INTEGER"}},"default": {"scalar": {"primitive": {"integer": "10"}}}}}}},"closure": {"expectedInputs": {"parameters": {"numbers": {"var": {"type": {"collectionType": {"simple": "INTEGER"}}}},"numbers_count": {"var": {"type": {"simple": "INTEGER"}}},"run_local_at_count": {"var": {"type": {"simple": "INTEGER"}},"default": {"scalar": {"primitive": {"integer": "10"}}}}}},"createdAt": "1970-01-01T00:00:01Z"}},{"id": {"name": "launchplan1","version": "v1"},"spec": {"defaultInputs": {"parameters": {"numbers": {"var": {"type": {"collectionType": {"simple": "INTEGER"}}}},"numbers_count": {"var": {"type": {"simple": "INTEGER"}}},"run_local_at_count": {"var": {"type": {"simple": "INTEGER"}},"default": {"scalar": {"primitive": {"integer": "10"}}}}}}},"closure": {"expectedInputs": {"parameters": {"numbers": {"var": {"type": {"collectionType": {"simple": "INTEGER"}}}},"numbers_count": {"var": {"type": {"simple": "INTEGER"}}},"run_local_at_count": {"var": {"type": {"simple": "INTEGER"}},"default": {"scalar": {"primitive": {"integer": "10"}}}}}},"createdAt": "1970-01-01T00:00:00Z"}}]`) + tearDownAndVerify(t, `[{"id": {"name": "launchplan1","version": "v2"},"spec": {"defaultInputs": {"parameters": {"numbers": {"var": {"type": {"collectionType": {"simple": "INTEGER"}},"description": "short desc"}},"numbers_count": {"var": {"type": {"simple": "INTEGER"},"description": "long description will be truncated in table"}},"run_local_at_count": {"var": {"type": {"simple": "INTEGER"},"description": "run_local_at_count"},"default": {"scalar": {"primitive": {"integer": "10"}}}}}}},"closure": {"expectedInputs": {"parameters": {"numbers": {"var": {"type": {"collectionType": {"simple": "INTEGER"}},"description": "short desc"}},"numbers_count": {"var": {"type": {"simple": "INTEGER"},"description": "long description will be truncated in table"}},"run_local_at_count": {"var": {"type": {"simple": "INTEGER"},"description": "run_local_at_count"},"default": {"scalar": {"primitive": {"integer": "10"}}}}}},"createdAt": "1970-01-01T00:00:01Z"}},{"id": {"name": "launchplan1","version": "v1"},"spec": {"defaultInputs": {"parameters": {"numbers": {"var": {"type": {"collectionType": {"simple": "INTEGER"}},"description": "short desc"}},"numbers_count": {"var": {"type": {"simple": "INTEGER"},"description": "long description will be truncated in table"}},"run_local_at_count": {"var": {"type": {"simple": "INTEGER"},"description": "run_local_at_count"},"default": {"scalar": {"primitive": {"integer": "10"}}}}}}},"closure": {"expectedInputs": {"parameters": {"numbers": {"var": {"type": {"collectionType": {"simple": "INTEGER"}},"description": "short desc"}},"numbers_count": {"var": {"type": {"simple": "INTEGER"},"description": "long description will be truncated in table"}},"run_local_at_count": {"var": {"type": {"simple": "INTEGER"},"description": "run_local_at_count"},"default": {"scalar": {"primitive": {"integer": "10"}}}}}},"createdAt": "1970-01-01T00:00:00Z"}}]`) } func TestGetLaunchPlansWithExecFile(t *testing.T) { @@ -624,21 +652,24 @@ func TestGetLaunchPlansWithExecFile(t *testing.T) { "collectionType": { "simple": "INTEGER" } - } + }, + "description": "short desc" } }, "numbers_count": { "var": { "type": { "simple": "INTEGER" - } + }, + "description": "long description will be truncated in table" } }, "run_local_at_count": { "var": { "type": { "simple": "INTEGER" - } + }, + "description": "run_local_at_count" }, "default": { "scalar": { @@ -660,21 +691,24 @@ func TestGetLaunchPlansWithExecFile(t *testing.T) { "collectionType": { "simple": "INTEGER" } - } + }, + "description": "short desc" } }, "numbers_count": { "var": { "type": { "simple": "INTEGER" - } + }, + "description": "long description will be truncated in table" } }, "run_local_at_count": { "var": { "type": { "simple": "INTEGER" - } + }, + "description": "run_local_at_count" }, "default": { "scalar": { @@ -690,3 +724,28 @@ func TestGetLaunchPlansWithExecFile(t *testing.T) { } }`) } + +func TestGetLaunchPlanTableFunc(t *testing.T) { + setup() + getLaunchPlanSetup() + mockClient.OnListLaunchPlansMatch(ctx, resourceGetRequest).Return(launchPlanListResponse, nil) + mockClient.OnGetLaunchPlanMatch(ctx, objectGetRequest).Return(launchPlan2, nil) + mockClient.OnListLaunchPlanIdsMatch(ctx, namedIDRequest).Return(namedIdentifierList, nil) + config.GetConfig().Output = "table" + err = getLaunchPlanFunc(ctx, argsLp, cmdCtx) + assert.Nil(t, err) + mockClient.AssertCalled(t, "ListLaunchPlans", ctx, resourceGetRequest) + tearDownAndVerify(t, ` +--------- ------------- ------ ------- ---------- --------------------------- --------- +| VERSION | NAME | TYPE | STATE | SCHEDULE | INPUTS | OUTPUTS | +--------- ------------- ------ ------- ---------- --------------------------- --------- +| v2 | launchplan1 | | | | numbers: short desc | | +| | | | | | numbers_count: long de... | | +| | | | | | run_local_at_count | | +--------- ------------- ------ ------- ---------- --------------------------- --------- +| v1 | launchplan1 | | | | numbers: short desc | | +| | | | | | numbers_count: long de... | | +| | | | | | run_local_at_count | | +--------- ------------- ------ ------- ---------- --------------------------- --------- +2 rows`) +} diff --git a/flytectl/cmd/get/task.go b/flytectl/cmd/get/task.go index 49c586f433..9f9a7b9861 100644 --- a/flytectl/cmd/get/task.go +++ b/flytectl/cmd/get/task.go @@ -10,7 +10,6 @@ import ( "github.com/flyteorg/flytectl/pkg/printer" "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin" "github.com/flyteorg/flytestdlib/logger" - "github.com/golang/protobuf/proto" ) @@ -98,6 +97,8 @@ var taskColumns = []printer.Column{ {Header: "Version", JSONPath: "$.id.version"}, {Header: "Name", JSONPath: "$.id.name"}, {Header: "Type", JSONPath: "$.closure.compiledTask.template.type"}, + {Header: "Inputs", JSONPath: "$.closure.compiledTask.template.interface.inputs.variables." + printer.DefaultFormattedDescriptionsKey + ".description"}, + {Header: "Outputs", JSONPath: "$.closure.compiledTask.template.interface.outputs.variables." + printer.DefaultFormattedDescriptionsKey + ".description"}, {Header: "Discoverable", JSONPath: "$.closure.compiledTask.template.metadata.discoverable"}, {Header: "Discovery Version", JSONPath: "$.closure.compiledTask.template.metadata.discoveryVersion"}, {Header: "Created At", JSONPath: "$.closure.createdAt"}, @@ -111,6 +112,21 @@ func TaskToProtoMessages(l []*admin.Task) []proto.Message { return messages } +func TaskToTableProtoMessages(l []*admin.Task) []proto.Message { + messages := make([]proto.Message, 0, len(l)) + for _, m := range l { + m := proto.Clone(m).(*admin.Task) + if m.Closure.CompiledTask.Template.Interface.Inputs != nil { + printer.FormatVariableDescriptions(m.Closure.CompiledTask.Template.Interface.Inputs.Variables) + } + if m.Closure.CompiledTask.Template.Interface.Outputs != nil { + printer.FormatVariableDescriptions(m.Closure.CompiledTask.Template.Interface.Outputs.Variables) + } + messages = append(messages, m) + } + return messages +} + func getTaskFunc(ctx context.Context, args []string, cmdCtx cmdCore.CommandContext) error { taskPrinter := printer.Printer{} var tasks []*admin.Task @@ -123,13 +139,20 @@ func getTaskFunc(ctx context.Context, args []string, cmdCtx cmdCore.CommandConte return err } logger.Debugf(ctx, "Retrieved Task", tasks) + if config.GetConfig().MustOutputFormat() == printer.OutputFormatTABLE { + return taskPrinter.Print(config.GetConfig().MustOutputFormat(), taskColumns, TaskToTableProtoMessages(tasks)...) + } return taskPrinter.Print(config.GetConfig().MustOutputFormat(), taskColumns, TaskToProtoMessages(tasks)...) + } tasks, err = cmdCtx.AdminFetcherExt().FetchAllVerOfTask(ctx, "", config.GetConfig().Project, config.GetConfig().Domain, taskConfig.DefaultConfig.Filter) if err != nil { return err } logger.Debugf(ctx, "Retrieved %v Task", len(tasks)) + if config.GetConfig().MustOutputFormat() == printer.OutputFormatTABLE { + return taskPrinter.Print(config.GetConfig().MustOutputFormat(), taskColumns, TaskToTableProtoMessages(tasks)...) + } return taskPrinter.Print(config.GetConfig().MustOutputFormat(), taskColumns, TaskToProtoMessages(tasks)...) } diff --git a/flytectl/cmd/get/task_test.go b/flytectl/cmd/get/task_test.go index b72545487d..744c56b29e 100644 --- a/flytectl/cmd/get/task_test.go +++ b/flytectl/cmd/get/task_test.go @@ -5,6 +5,8 @@ import ( "os" "testing" + "github.com/flyteorg/flytectl/cmd/config" + taskConfig "github.com/flyteorg/flytectl/cmd/config/subcommand/task" "github.com/flyteorg/flytectl/pkg/filters" @@ -50,6 +52,7 @@ func getTaskSetup() { }, }, }, + Description: "var description", } variableMap := map[string]*core.Variable{ "sorted_list1": &sortedListLiteralType, @@ -257,14 +260,16 @@ func TestGetTaskFunc(t *testing.T) { "collectionType": { "simple": "INTEGER" } - } + }, + "description": "var description" }, "sorted_list2": { "type": { "collectionType": { "simple": "INTEGER" } - } + }, + "description": "var description" } } } @@ -290,14 +295,16 @@ func TestGetTaskFunc(t *testing.T) { "collectionType": { "simple": "INTEGER" } - } + }, + "description": "var description" }, "sorted_list2": { "type": { "collectionType": { "simple": "INTEGER" } - } + }, + "description": "var description" } } } @@ -310,6 +317,28 @@ func TestGetTaskFunc(t *testing.T) { ]`) } +func TestGetTaskTableFunc(t *testing.T) { + setup() + getTaskSetup() + mockClient.OnListTasksMatch(ctx, resourceListRequestTask).Return(taskListResponse, nil) + mockClient.OnGetTaskMatch(ctx, objectGetRequestTask).Return(task2, nil) + config.GetConfig().Output = "table" + err = getTaskFunc(ctx, argsTask, cmdCtx) + assert.Nil(t, err) + mockClient.AssertCalled(t, "ListTasks", ctx, resourceListRequestTask) + tearDownAndVerify(t, ` +--------- ------- ------ --------------------------- --------- -------------- ------------------- ---------------------- +| VERSION | NAME | TYPE | INPUTS | OUTPUTS | DISCOVERABLE | DISCOVERY VERSION | CREATED AT | +--------- ------- ------ --------------------------- --------- -------------- ------------------- ---------------------- +| v2 | task1 | | sorted_list1: var desc... | | | | 1970-01-01T00:00:01Z | +| | | | sorted_list2: var desc... | | | | | +--------- ------- ------ --------------------------- --------- -------------- ------------------- ---------------------- +| v1 | task1 | | sorted_list1: var desc... | | | | 1970-01-01T00:00:00Z | +| | | | sorted_list2: var desc... | | | | | +--------- ------- ------ --------------------------- --------- -------------- ------------------- ---------------------- +2 rows`) +} + func TestGetTaskFuncLatest(t *testing.T) { setup() getTaskSetup() @@ -336,14 +365,16 @@ func TestGetTaskFuncLatest(t *testing.T) { "collectionType": { "simple": "INTEGER" } - } + }, + "description": "var description" }, "sorted_list2": { "type": { "collectionType": { "simple": "INTEGER" } - } + }, + "description": "var description" } } } @@ -382,14 +413,16 @@ func TestGetTaskWithVersion(t *testing.T) { "collectionType": { "simple": "INTEGER" } - } + }, + "description": "var description" }, "sorted_list2": { "type": { "collectionType": { "simple": "INTEGER" } - } + }, + "description": "var description" } } } @@ -408,7 +441,7 @@ func TestGetTasks(t *testing.T) { mockClient.OnGetTaskMatch(ctx, objectGetRequestTask).Return(task2, nil) err = getTaskFunc(ctx, argsTask, cmdCtx) assert.Nil(t, err) - tearDownAndVerify(t, `[{"id": {"name": "task1","version": "v2"},"closure": {"compiledTask": {"template": {"interface": {"inputs": {"variables": {"sorted_list1": {"type": {"collectionType": {"simple": "INTEGER"}}},"sorted_list2": {"type": {"collectionType": {"simple": "INTEGER"}}}}}}}},"createdAt": "1970-01-01T00:00:01Z"}},{"id": {"name": "task1","version": "v1"},"closure": {"compiledTask": {"template": {"interface": {"inputs": {"variables": {"sorted_list1": {"type": {"collectionType": {"simple": "INTEGER"}}},"sorted_list2": {"type": {"collectionType": {"simple": "INTEGER"}}}}}}}},"createdAt": "1970-01-01T00:00:00Z"}}]`) + tearDownAndVerify(t, `[{"id": {"name": "task1","version": "v2"},"closure": {"compiledTask": {"template": {"interface": {"inputs": {"variables": {"sorted_list1": {"type": {"collectionType": {"simple": "INTEGER"}},"description": "var description"},"sorted_list2": {"type": {"collectionType": {"simple": "INTEGER"}},"description": "var description"}}}}}},"createdAt": "1970-01-01T00:00:01Z"}},{"id": {"name": "task1","version": "v1"},"closure": {"compiledTask": {"template": {"interface": {"inputs": {"variables": {"sorted_list1": {"type": {"collectionType": {"simple": "INTEGER"}},"description": "var description"},"sorted_list2": {"type": {"collectionType": {"simple": "INTEGER"}},"description": "var description"}}}}}},"createdAt": "1970-01-01T00:00:00Z"}}]`) } func TestGetTasksFilters(t *testing.T) { @@ -420,7 +453,7 @@ func TestGetTasksFilters(t *testing.T) { mockClient.OnListTasksMatch(ctx, resourceListFilterRequestTask).Return(taskListFilterResponse, nil) err = getTaskFunc(ctx, argsTask, cmdCtx) assert.Nil(t, err) - tearDownAndVerify(t, `{"id": {"name": "task1","version": "v1"},"closure": {"compiledTask": {"template": {"interface": {"inputs": {"variables": {"sorted_list1": {"type": {"collectionType": {"simple": "INTEGER"}}},"sorted_list2": {"type": {"collectionType": {"simple": "INTEGER"}}}}}}}},"createdAt": "1970-01-01T00:00:00Z"}}`) + tearDownAndVerify(t, `{"id": {"name": "task1","version": "v1"},"closure": {"compiledTask": {"template": {"interface": {"inputs": {"variables": {"sorted_list1": {"type": {"collectionType": {"simple": "INTEGER"}},"description": "var description"},"sorted_list2": {"type": {"collectionType": {"simple": "INTEGER"}},"description": "var description"}}}}}},"createdAt": "1970-01-01T00:00:00Z"}}`) } func TestGetTaskWithExecFile(t *testing.T) { @@ -451,14 +484,16 @@ func TestGetTaskWithExecFile(t *testing.T) { "collectionType": { "simple": "INTEGER" } - } + }, + "description": "var description" }, "sorted_list2": { "type": { "collectionType": { "simple": "INTEGER" } - } + }, + "description": "var description" } } } diff --git a/flytectl/cmd/get/workflow.go b/flytectl/cmd/get/workflow.go index 8fba30d2cd..704f7a0519 100644 --- a/flytectl/cmd/get/workflow.go +++ b/flytectl/cmd/get/workflow.go @@ -87,6 +87,8 @@ Usage var workflowColumns = []printer.Column{ {Header: "Version", JSONPath: "$.id.version"}, {Header: "Name", JSONPath: "$.id.name"}, + {Header: "Inputs", JSONPath: "$.closure.compiledWorkflow.primary.template.interface.inputs.variables." + printer.DefaultFormattedDescriptionsKey + ".description"}, + {Header: "Outputs", JSONPath: "$.closure.compiledWorkflow.primary.template.interface.outputs.variables." + printer.DefaultFormattedDescriptionsKey + ".description"}, {Header: "Created At", JSONPath: "$.closure.createdAt"}, } @@ -98,6 +100,21 @@ func WorkflowToProtoMessages(l []*admin.Workflow) []proto.Message { return messages } +func WorkflowToTableProtoMessages(l []*admin.Workflow) []proto.Message { + messages := make([]proto.Message, 0, len(l)) + for _, m := range l { + m := proto.Clone(m).(*admin.Workflow) + if m.Closure.CompiledWorkflow.Primary.Template.Interface.Inputs != nil { + printer.FormatVariableDescriptions(m.Closure.CompiledWorkflow.Primary.Template.Interface.Inputs.Variables) + } + if m.Closure.CompiledWorkflow.Primary.Template.Interface.Outputs != nil { + printer.FormatVariableDescriptions(m.Closure.CompiledWorkflow.Primary.Template.Interface.Outputs.Variables) + } + messages = append(messages, m) + } + return messages +} + func getWorkflowFunc(ctx context.Context, args []string, cmdCtx cmdCore.CommandContext) error { adminPrinter := printer.Printer{} var workflows []*admin.Workflow @@ -108,6 +125,9 @@ func getWorkflowFunc(ctx context.Context, args []string, cmdCtx cmdCore.CommandC return err } logger.Debugf(ctx, "Retrieved %v workflow", len(workflows)) + if config.GetConfig().MustOutputFormat() == printer.OutputFormatTABLE { + return adminPrinter.Print(config.GetConfig().MustOutputFormat(), workflowColumns, WorkflowToTableProtoMessages(workflows)...) + } return adminPrinter.Print(config.GetConfig().MustOutputFormat(), workflowColumns, WorkflowToProtoMessages(workflows)...) } @@ -117,6 +137,9 @@ func getWorkflowFunc(ctx context.Context, args []string, cmdCtx cmdCore.CommandC } logger.Debugf(ctx, "Retrieved %v workflows", len(workflows)) + if config.GetConfig().MustOutputFormat() == printer.OutputFormatTABLE { + return adminPrinter.Print(config.GetConfig().MustOutputFormat(), workflowColumns, WorkflowToTableProtoMessages(workflows)...) + } return adminPrinter.Print(config.GetConfig().MustOutputFormat(), workflowColumns, WorkflowToProtoMessages(workflows)...) } diff --git a/flytectl/cmd/testutils/test_utils.go b/flytectl/cmd/testutils/test_utils.go index f386341f32..c04be45a52 100644 --- a/flytectl/cmd/testutils/test_utils.go +++ b/flytectl/cmd/testutils/test_utils.go @@ -6,6 +6,7 @@ import ( "io" "log" "os" + "regexp" "strings" "testing" @@ -68,10 +69,12 @@ func TearDownAndVerify(t *testing.T, expectedLog string) { os.Stderr = stderr var buf bytes.Buffer if _, err := io.Copy(&buf, reader); err == nil { - assert.Equal(t, santizeString(expectedLog), santizeString(buf.String())) + assert.Equal(t, sanitizeString(expectedLog), sanitizeString(buf.String())) } } -func santizeString(str string) string { - return strings.Trim(strings.ReplaceAll(strings.ReplaceAll(str, "\n", ""), "\t", ""), " \t") +func sanitizeString(str string) string { + // Not the most comprehensive ANSI pattern, but this should capture common color operations such as \x1b[107;0m and \x1b[0m. Expand if needed (insert regex 2 problems joke here). + ansiRegex := regexp.MustCompile("\u001B\\[[\\d+\\;]*\\d+m") + return ansiRegex.ReplaceAllString(strings.Trim(strings.ReplaceAll(strings.ReplaceAll(strings.ReplaceAll(str, "\n", ""), "\t", ""), "", ""), " \t"), "") } diff --git a/flytectl/go.mod b/flytectl/go.mod index 00186a8e76..438cc0c3b4 100644 --- a/flytectl/go.mod +++ b/flytectl/go.mod @@ -42,6 +42,7 @@ require ( google.golang.org/protobuf v1.25.0 gopkg.in/check.v1 v1.0.0-20200227125254-8fa46927fb4f // indirect gopkg.in/yaml.v2 v2.4.0 + gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b // indirect gotest.tools v2.2.0+incompatible sigs.k8s.io/yaml v1.2.0 ) diff --git a/flytectl/pkg/printer/printer.go b/flytectl/pkg/printer/printer.go index 2afcff5403..75d67208eb 100644 --- a/flytectl/pkg/printer/printer.go +++ b/flytectl/pkg/printer/printer.go @@ -6,6 +6,10 @@ import ( "fmt" "net/url" "os" + "sort" + "strings" + + "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/core" "github.com/flyteorg/flytectl/pkg/visualize" "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin" @@ -51,8 +55,10 @@ type Column struct { type Printer struct{} const ( - empty = "" - tab = "\t" + empty = "" + tab = "\t" + DefaultFormattedDescriptionsKey = "_formatted_descriptions" + defaultLineWidth = 25 ) // Projects the columns in one row of data from the given JSON using the []Column map @@ -164,6 +170,61 @@ func printJSONYaml(format OutputFormat, v interface{}) error { return nil } +func FormatVariableDescriptions(variableMap map[string]*core.Variable) { + keys := make([]string, 0, len(variableMap)) + // sort the keys for testing and consistency with other output formats + for k := range variableMap { + keys = append(keys, k) + } + sort.Strings(keys) + + var descriptions []string + for _, k := range keys { + v := variableMap[k] + // a: a isn't very helpful + if k != v.Description { + descriptions = append(descriptions, getTruncatedLine(fmt.Sprintf("%s: %s", k, v.Description))) + } else { + descriptions = append(descriptions, getTruncatedLine(k)) + } + + } + variableMap[DefaultFormattedDescriptionsKey] = &core.Variable{Description: strings.Join(descriptions, "\n")} +} + +func FormatParameterDescriptions(parameterMap map[string]*core.Parameter) { + keys := make([]string, 0, len(parameterMap)) + // sort the keys for testing and consistency with other output formats + for k := range parameterMap { + keys = append(keys, k) + } + sort.Strings(keys) + + var descriptions []string + for _, k := range keys { + v := parameterMap[k] + if v.Var == nil { + continue + } + // a: a isn't very helpful + if k != v.Var.Description { + descriptions = append(descriptions, getTruncatedLine(fmt.Sprintf("%s: %s", k, v.Var.Description))) + } else { + descriptions = append(descriptions, getTruncatedLine(k)) + } + } + parameterMap[DefaultFormattedDescriptionsKey] = &core.Parameter{Var: &core.Variable{Description: strings.Join(descriptions, "\n")}} +} + +func getTruncatedLine(line string) string { + // TODO: maybe add width to function signature later + width := defaultLineWidth + if len(line) > width { + return line[:width-3] + "..." + } + return line +} + func (p Printer) Print(format OutputFormat, columns []Column, messages ...proto.Message) error { printableMessages := make([]*PrintableProto, 0, len(messages)) diff --git a/flytectl/pkg/printer/printer_test.go b/flytectl/pkg/printer/printer_test.go index faa11a1978..6d5441b9af 100644 --- a/flytectl/pkg/printer/printer_test.go +++ b/flytectl/pkg/printer/printer_test.go @@ -253,3 +253,57 @@ func TestPrint(t *testing.T) { assert.NotNil(t, err) assert.Equal(t, fmt.Errorf("no template found in the sub workflow template:<> "), errors.Unwrap(err)) } + +func TestGetTruncatedLine(t *testing.T) { + testStrings := map[string]string{ + "foo": "foo", + "": "", + "short description": "short description", + "1234567890123456789012345": "1234567890123456789012345", + "12345678901234567890123456": "1234567890123456789012...", + "long description probably needs truncate": "long description proba...", + } + for k, v := range testStrings { + assert.Equal(t, v, getTruncatedLine(k)) + } +} + +func TestFormatVariableDescriptions(t *testing.T) { + fooVar := &core.Variable{ + Description: "foo", + } + barVar := &core.Variable{ + Description: "bar", + } + variableMap := map[string]*core.Variable{ + "var1": fooVar, + "var2": barVar, + "foo": fooVar, + "bar": barVar, + } + FormatVariableDescriptions(variableMap) + assert.Equal(t, "bar\nfoo\nvar1: foo\nvar2: bar", variableMap[DefaultFormattedDescriptionsKey].Description) +} + +func TestFormatParameterDescriptions(t *testing.T) { + fooParam := &core.Parameter{ + Var: &core.Variable{ + Description: "foo", + }, + } + barParam := &core.Parameter{ + Var: &core.Variable{ + Description: "bar", + }, + } + emptyParam := &core.Parameter{} + paramMap := map[string]*core.Parameter{ + "var1": fooParam, + "var2": barParam, + "foo": fooParam, + "bar": barParam, + "empty": emptyParam, + } + FormatParameterDescriptions(paramMap) + assert.Equal(t, "bar\nfoo\nvar1: foo\nvar2: bar", paramMap[DefaultFormattedDescriptionsKey].Var.Description) +}