Refactor get command (flyteorg#84)

* Refactor get command Signed-off-by: Yuvraj <[email protected]> * Fix unit test Signed-off-by: Yuvraj <[email protected]>
eapolinario · Jun 7, 2021 · 052316d · 052316d
1 parent e2082c0
commit 052316d
Show file tree

Hide file tree

Showing 12 changed files with 209 additions and 82 deletions.
diff --git a/flytectl/cmd/get/execution.go b/flytectl/cmd/get/execution.go
@@ -3,8 +3,6 @@ package get
 import (
 	"context"
 
-	"github.com/flyteorg/flytectl/pkg/filters"
-
 	"github.com/flyteorg/flytectl/cmd/config"
 	"github.com/flyteorg/flytectl/cmd/config/subcommand/execution"
 	cmdCore "github.com/flyteorg/flytectl/cmd/core"
@@ -85,18 +83,15 @@ func getExecutionFunc(ctx context.Context, args []string, cmdCtx cmdCore.Command
 			return err
 		}
 		executions = append(executions, execution)
-	} else {
-		transformFilters, err := filters.BuildResourceListRequestWithName(execution.DefaultConfig.Filter, config.GetConfig().Project, config.GetConfig().Domain, "")
-		if err != nil {
-			return err
-		}
-		executionList, err := cmdCtx.AdminClient().ListExecutions(ctx, transformFilters)
-		if err != nil {
-			return err
-		}
-		executions = executionList.Executions
+		logger.Infof(ctx, "Retrieved %v executions", len(executions))
+		return adminPrinter.Print(config.GetConfig().MustOutputFormat(), executionColumns,
+			ExecutionToProtoMessages(executions)...)
+	}
+	executionList, err := cmdCtx.AdminFetcherExt().ListExecution(ctx, config.GetConfig().Project, config.GetConfig().Domain, execution.DefaultConfig.Filter)
+	if err != nil {
+		return err
 	}
-	logger.Infof(ctx, "Retrieved %v executions", len(executions))
+	logger.Infof(ctx, "Retrieved %v executions", len(executionList.Executions))
 	return adminPrinter.Print(config.GetConfig().MustOutputFormat(), executionColumns,
-		ExecutionToProtoMessages(executions)...)
+		ExecutionToProtoMessages(executionList.Executions)...)
 }
diff --git a/flytectl/cmd/get/launch_plan.go b/flytectl/cmd/get/launch_plan.go
@@ -3,8 +3,6 @@ package get
 import (
 	"context"
 
-	"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"
@@ -126,11 +124,11 @@ func LaunchplanToProtoMessages(l []*admin.LaunchPlan) []proto.Message {
 
 func getLaunchPlanFunc(ctx context.Context, args []string, cmdCtx cmdCore.CommandContext) error {
 	launchPlanPrinter := printer.Printer{}
+	var launchPlans []*admin.LaunchPlan
 	project := config.GetConfig().Project
 	domain := config.GetConfig().Domain
 	if len(args) == 1 {
 		name := args[0]
-		var launchPlans []*admin.LaunchPlan
 		var err error
 		if launchPlans, err = FetchLPForName(ctx, cmdCtx.AdminFetcherExt(), name, project, domain); err != nil {
 			return err
@@ -143,15 +141,12 @@ func getLaunchPlanFunc(ctx context.Context, args []string, cmdCtx cmdCore.Comman
 		}
 		return nil
 	}
-	transformFilters, err := filters.BuildResourceListRequestWithName(launchplan.DefaultConfig.Filter, config.GetConfig().Project, config.GetConfig().Domain, "")
-	if err != nil {
-		return err
-	}
-	launchPlanList, err := cmdCtx.AdminClient().ListLaunchPlans(ctx, transformFilters)
+
+	launchPlans, err := cmdCtx.AdminFetcherExt().FetchAllVerOfLP(ctx, "", config.GetConfig().Project, config.GetConfig().Domain, launchplan.DefaultConfig.Filter)
 	if err != nil {
 		return err
 	}
-	launchPlans := launchPlanList.LaunchPlans
+
 	logger.Debugf(ctx, "Retrieved %v launch plans", len(launchPlans))
 	return launchPlanPrinter.Print(config.GetConfig().MustOutputFormat(), launchplansColumns,
 		LaunchplanToProtoMessages(launchPlans)...)

diff --git a/flytectl/cmd/get/project.go b/flytectl/cmd/get/project.go
@@ -5,8 +5,6 @@ import (
 
 	"github.com/flyteorg/flytectl/cmd/config/subcommand/project"
 
-	"github.com/flyteorg/flytectl/pkg/filters"
-
 	"github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin"
 	"github.com/flyteorg/flytestdlib/logger"
 	"github.com/golang/protobuf/proto"
@@ -72,11 +70,8 @@ func ProjectToProtoMessages(l []*admin.Project) []proto.Message {
 
 func getProjectsFunc(ctx context.Context, args []string, cmdCtx cmdCore.CommandContext) error {
 	adminPrinter := printer.Printer{}
-	transformFilters, err := filters.BuildProjectListRequest(project.DefaultConfig.Filter)
-	if err != nil {
-		return err
-	}
-	projects, err := cmdCtx.AdminClient().ListProjects(ctx, transformFilters)
+
+	projects, err := cmdCtx.AdminFetcherExt().ListProjects(ctx, project.DefaultConfig.Filter)
 	if err != nil {
 		return err
 	}

diff --git a/flytectl/cmd/get/project_test.go b/flytectl/cmd/get/project_test.go
@@ -2,11 +2,15 @@ package get
 
 import (
 	"fmt"
+	"io"
 	"testing"
 
+	cmdCore "github.com/flyteorg/flytectl/cmd/core"
+
 	"github.com/flyteorg/flytectl/cmd/config/subcommand/project"
 
 	"github.com/flyteorg/flytectl/pkg/filters"
+	"github.com/flyteorg/flyteidl/clients/go/admin/mocks"
 	"github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin"
 	"github.com/stretchr/testify/assert"
 )
@@ -19,6 +23,10 @@ var (
 )
 
 func getProjectSetup() {
+
+	mockOutStream := new(io.Writer)
+	cmdCtx = cmdCore.NewCommandContext(mockClient, *mockOutStream)
+
 	argsProject = []string{"flyteexample"}
 	resourceListRequestProject = &admin.ProjectListRequest{}
 
@@ -51,19 +59,27 @@ func getProjectSetup() {
 	}
 }
 
-func TestProjectFunc(t *testing.T) {
+func TestListProjectFunc(t *testing.T) {
 	setup()
 	getProjectSetup()
+	mockClient := new(mocks.AdminServiceClient)
+	mockOutStream := new(io.Writer)
+	cmdCtx := cmdCore.NewCommandContext(mockClient, *mockOutStream)
+
 	project.DefaultConfig.Filter = filters.Filters{}
 	mockClient.OnListProjectsMatch(ctx, resourceListRequestProject).Return(projectListResponse, nil)
 	err = getProjectsFunc(ctx, argsProject, cmdCtx)
+
 	assert.Nil(t, err)
 	mockClient.AssertCalled(t, "ListProjects", ctx, resourceListRequestProject)
 }
 
 func TestGetProjectFunc(t *testing.T) {
 	setup()
 	getProjectSetup()
+
+	argsProject = []string{}
+
 	project.DefaultConfig.Filter = filters.Filters{}
 	mockClient.OnListProjectsMatch(ctx, resourceListRequestProject).Return(projectListResponse, nil)
 	err = getProjectsFunc(ctx, argsProject, cmdCtx)

diff --git a/flytectl/cmd/get/task.go b/flytectl/cmd/get/task.go
@@ -7,7 +7,6 @@ import (
 	taskConfig "github.com/flyteorg/flytectl/cmd/config/subcommand/task"
 	cmdCore "github.com/flyteorg/flytectl/cmd/core"
 	"github.com/flyteorg/flytectl/pkg/ext"
-	"github.com/flyteorg/flytectl/pkg/filters"
 	"github.com/flyteorg/flytectl/pkg/printer"
 	"github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin"
 	"github.com/flyteorg/flytestdlib/logger"
@@ -114,28 +113,22 @@ func TaskToProtoMessages(l []*admin.Task) []proto.Message {
 
 func getTaskFunc(ctx context.Context, args []string, cmdCtx cmdCore.CommandContext) error {
 	taskPrinter := printer.Printer{}
+	var tasks []*admin.Task
+	var err error
 	project := config.GetConfig().Project
 	domain := config.GetConfig().Domain
 	if len(args) == 1 {
 		name := args[0]
-		var tasks []*admin.Task
-		var err error
 		if tasks, err = FetchTaskForName(ctx, cmdCtx.AdminFetcherExt(), name, project, domain); err != nil {
 			return err
 		}
 		logger.Debugf(ctx, "Retrieved Task", tasks)
 		return taskPrinter.Print(config.GetConfig().MustOutputFormat(), taskColumns, TaskToProtoMessages(tasks)...)
 	}
-	transformFilters, err := filters.BuildResourceListRequestWithName(taskConfig.DefaultConfig.Filter, config.GetConfig().Project, config.GetConfig().Domain, "")
-	if err != nil {
-		return err
-	}
-	taskList, err := cmdCtx.AdminClient().ListTasks(ctx, transformFilters)
+	tasks, err = cmdCtx.AdminFetcherExt().FetchAllVerOfTask(ctx, "", config.GetConfig().Project, config.GetConfig().Domain, taskConfig.DefaultConfig.Filter)
 	if err != nil {
 		return err
 	}
-	tasks := taskList.Tasks
-
 	logger.Debugf(ctx, "Retrieved %v Task", len(tasks))
 	return taskPrinter.Print(config.GetConfig().MustOutputFormat(), taskColumns, TaskToProtoMessages(tasks)...)
 }

diff --git a/flytectl/cmd/get/workflow.go b/flytectl/cmd/get/workflow.go
@@ -3,8 +3,6 @@ package get
 import (
 	"context"
 
-	"github.com/flyteorg/flytectl/pkg/filters"
-
 	workflowconfig "github.com/flyteorg/flytectl/cmd/config/subcommand/workflow"
 	"github.com/flyteorg/flytectl/pkg/ext"
 	"github.com/flyteorg/flytestdlib/logger"
@@ -74,18 +72,6 @@ Usage
 `
 )
 
-//go:generate pflags WorkflowConfig --default-var workflowConfig
-var (
-	workflowConfig = &WorkflowConfig{
-		Filter: filters.DefaultFilter,
-	}
-)
-
-// WorkflowConfig
-type WorkflowConfig struct {
-	Filter filters.Filters `json:"filter" pflag:","`
-}
-
 var workflowColumns = []printer.Column{
 	{Header: "Version", JSONPath: "$.id.version"},
 	{Header: "Name", JSONPath: "$.id.name"},
@@ -102,30 +88,21 @@ func WorkflowToProtoMessages(l []*admin.Workflow) []proto.Message {
 
 func getWorkflowFunc(ctx context.Context, args []string, cmdCtx cmdCore.CommandContext) error {
 	adminPrinter := printer.Printer{}
+	var workflows []*admin.Workflow
+	var err error
 	if len(args) > 0 {
 		name := args[0]
-		var workflows []*admin.Workflow
-		var err error
 		if workflows, err = FetchWorkflowForName(ctx, cmdCtx.AdminFetcherExt(), name, config.GetConfig().Project, config.GetConfig().Domain); err != nil {
 			return err
 		}
 		logger.Debugf(ctx, "Retrieved %v workflow", len(workflows))
-		err = adminPrinter.Print(config.GetConfig().MustOutputFormat(), workflowColumns, WorkflowToProtoMessages(workflows)...)
-		if err != nil {
-			return err
-		}
-		return nil
+		return adminPrinter.Print(config.GetConfig().MustOutputFormat(), workflowColumns, WorkflowToProtoMessages(workflows)...)
 	}
 
-	transformFilters, err := filters.BuildResourceListRequestWithName(workflowConfig.Filter, config.GetConfig().Project, config.GetConfig().Domain, "")
-	if err != nil {
-		return err
-	}
-	workflowList, err := cmdCtx.AdminClient().ListWorkflows(ctx, transformFilters)
+	workflows, err = cmdCtx.AdminFetcherExt().FetchAllVerOfWorkflow(ctx, "", config.GetConfig().Project, config.GetConfig().Domain, workflowconfig.DefaultConfig.Filter)
 	if err != nil {
 		return err
 	}
-	workflows := workflowList.Workflows
 
 	logger.Debugf(ctx, "Retrieved %v workflows", len(workflows))
 	return adminPrinter.Print(config.GetConfig().MustOutputFormat(), workflowColumns, WorkflowToProtoMessages(workflows)...)
@@ -138,7 +115,7 @@ func FetchWorkflowForName(ctx context.Context, fetcher ext.AdminFetcherExtInterf
 	var workflow *admin.Workflow
 	var err error
 	if workflowconfig.DefaultConfig.Latest {
-		if workflow, err = fetcher.FetchWorkflowLatestVersion(ctx, name, project, domain, workflowConfig.Filter); err != nil {
+		if workflow, err = fetcher.FetchWorkflowLatestVersion(ctx, name, project, domain, workflowconfig.DefaultConfig.Filter); err != nil {
 			return nil, err
 		}
 		workflows = append(workflows, workflow)
@@ -148,7 +125,7 @@ func FetchWorkflowForName(ctx context.Context, fetcher ext.AdminFetcherExtInterf
 		}
 		workflows = append(workflows, workflow)
 	} else {
-		workflows, err = fetcher.FetchAllVerOfWorkflow(ctx, name, project, domain, workflowConfig.Filter)
+		workflows, err = fetcher.FetchAllVerOfWorkflow(ctx, name, project, domain, workflowconfig.DefaultConfig.Filter)
 		if err != nil {
 			return nil, err
 		}

diff --git a/flytectl/cmd/get/workflow_test.go b/flytectl/cmd/get/workflow_test.go
@@ -5,7 +5,6 @@ import (
 	"testing"
 
 	"github.com/flyteorg/flytectl/pkg/ext/mocks"
-	"github.com/flyteorg/flytectl/pkg/filters"
 	"github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin"
 	"github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/core"
 	"github.com/stretchr/testify/assert"
@@ -97,14 +96,3 @@ func TestGetWorkflowFuncWithError(t *testing.T) {
 	})
 
 }
-
-func TestGetWorkflowFunc(t *testing.T) {
-	setup()
-	getWorkflowSetup()
-	workflowConfig.Filter = filters.Filters{}
-	argsWorkflow := []string{}
-	mockClient.OnListWorkflowsMatch(ctx, resourceListRequestWorkflow).Return(workflowListResponse, nil)
-	err = getWorkflowFunc(ctx, argsWorkflow, cmdCtx)
-	assert.Nil(t, err)
-	mockClient.AssertCalled(t, "ListWorkflows", ctx, resourceListRequestWorkflow)
-}
diff --git a/flytectl/pkg/ext/execution_fetcher.go b/flytectl/pkg/ext/execution_fetcher.go
@@ -3,6 +3,8 @@ package ext
 import (
 	"context"
 
+	"github.com/flyteorg/flytectl/pkg/filters"
+
 	"github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin"
 	"github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/core"
 )
@@ -20,3 +22,15 @@ func (a *AdminFetcherExtClient) FetchExecution(ctx context.Context, name, projec
 	}
 	return e, nil
 }
+
+func (a *AdminFetcherExtClient) ListExecution(ctx context.Context, project, domain string, filter filters.Filters) (*admin.ExecutionList, error) {
+	transformFilters, err := filters.BuildResourceListRequestWithName(filter, project, domain, "")
+	if err != nil {
+		return nil, err
+	}
+	e, err := a.AdminServiceClient().ListExecutions(ctx, transformFilters)
+	if err != nil {
+		return nil, err
+	}
+	return e, nil
+}
diff --git a/flytectl/pkg/ext/fetcher.go b/flytectl/pkg/ext/fetcher.go
@@ -19,6 +19,9 @@ type AdminFetcherExtInterface interface {
 	// FetchExecution fetches the execution based on name, project, domain
 	FetchExecution(ctx context.Context, name, project, domain string) (*admin.Execution, error)
 
+	// ListExecution fetches the all versions of  based on name, project, domain
+	ListExecution(ctx context.Context, project, domain string, filter filters.Filters) (*admin.ExecutionList, error)
+
 	// FetchAllVerOfLP fetches all versions of launch plan in a  project, domain
 	FetchAllVerOfLP(ctx context.Context, lpName, project, domain string, filter filters.Filters) ([]*admin.LaunchPlan, error)
 
@@ -51,6 +54,9 @@ type AdminFetcherExtInterface interface {
 
 	// FetchProjectDomainAttributes fetches project domain attributes particular resource type in a  project, domain
 	FetchProjectDomainAttributes(ctx context.Context, project, domain string, rsType admin.MatchableResource) (*admin.ProjectDomainAttributesGetResponse, error)
+
+	// ListProjects fetches all projects
+	ListProjects(ctx context.Context, filter filters.Filters) (*admin.Projects, error)
 }
 
 // AdminFetcherExtClient is used for interacting with extended features used for fetching data from admin service