From f4d0b8347499812567b37f8fe37bb503587a0711 Mon Sep 17 00:00:00 2001 From: Yuvraj Date: Mon, 7 Jun 2021 11:33:57 +0530 Subject: [PATCH] Refactor get command (#84) * Refactor get command Signed-off-by: Yuvraj * Fix unit test Signed-off-by: Yuvraj --- cmd/get/execution.go | 23 +++--- cmd/get/launch_plan.go | 13 +--- cmd/get/project.go | 9 +-- cmd/get/project_test.go | 18 ++++- cmd/get/task.go | 13 +--- cmd/get/workflow.go | 35 ++------- cmd/get/workflow_test.go | 12 --- pkg/ext/execution_fetcher.go | 14 ++++ pkg/ext/fetcher.go | 6 ++ pkg/ext/mocks/admin_fetcher_ext_interface.go | 82 ++++++++++++++++++++ pkg/ext/project_fetcher.go | 21 +++++ pkg/ext/project_fetcher_test.go | 45 +++++++++++ 12 files changed, 209 insertions(+), 82 deletions(-) create mode 100644 pkg/ext/project_fetcher.go create mode 100644 pkg/ext/project_fetcher_test.go diff --git a/cmd/get/execution.go b/cmd/get/execution.go index 73c2e36b78..e7164ca90e 100644 --- a/cmd/get/execution.go +++ b/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/cmd/get/launch_plan.go b/cmd/get/launch_plan.go index 9619bd1fde..9edff6ab92 100644 --- a/cmd/get/launch_plan.go +++ b/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/cmd/get/project.go b/cmd/get/project.go index 535cb16af4..4d13950e1e 100644 --- a/cmd/get/project.go +++ b/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/cmd/get/project_test.go b/cmd/get/project_test.go index 2419ceda6c..264aa35c1c 100644 --- a/cmd/get/project_test.go +++ b/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,12 +59,17 @@ 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) } @@ -64,6 +77,9 @@ func TestProjectFunc(t *testing.T) { 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/cmd/get/task.go b/cmd/get/task.go index fd02d17b40..e25206a55f 100644 --- a/cmd/get/task.go +++ b/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/cmd/get/workflow.go b/cmd/get/workflow.go index c87cc3406b..cdefe89410 100644 --- a/cmd/get/workflow.go +++ b/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/cmd/get/workflow_test.go b/cmd/get/workflow_test.go index 29f3274afc..a3e47e8332 100644 --- a/cmd/get/workflow_test.go +++ b/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/pkg/ext/execution_fetcher.go b/pkg/ext/execution_fetcher.go index e96351c8b8..4e30a708bb 100644 --- a/pkg/ext/execution_fetcher.go +++ b/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/pkg/ext/fetcher.go b/pkg/ext/fetcher.go index ecc3b6371c..83a4753401 100644 --- a/pkg/ext/fetcher.go +++ b/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 diff --git a/pkg/ext/mocks/admin_fetcher_ext_interface.go b/pkg/ext/mocks/admin_fetcher_ext_interface.go index 8ae7cde8f8..23b64e3696 100644 --- a/pkg/ext/mocks/admin_fetcher_ext_interface.go +++ b/pkg/ext/mocks/admin_fetcher_ext_interface.go @@ -544,3 +544,85 @@ func (_m *AdminFetcherExtInterface) FetchWorkflowVersion(ctx context.Context, na return r0, r1 } + +type AdminFetcherExtInterface_ListExecution struct { + *mock.Call +} + +func (_m AdminFetcherExtInterface_ListExecution) Return(_a0 *admin.ExecutionList, _a1 error) *AdminFetcherExtInterface_ListExecution { + return &AdminFetcherExtInterface_ListExecution{Call: _m.Call.Return(_a0, _a1)} +} + +func (_m *AdminFetcherExtInterface) OnListExecution(ctx context.Context, project string, domain string, filter filters.Filters) *AdminFetcherExtInterface_ListExecution { + c := _m.On("ListExecution", ctx, project, domain, filter) + return &AdminFetcherExtInterface_ListExecution{Call: c} +} + +func (_m *AdminFetcherExtInterface) OnListExecutionMatch(matchers ...interface{}) *AdminFetcherExtInterface_ListExecution { + c := _m.On("ListExecution", matchers...) + return &AdminFetcherExtInterface_ListExecution{Call: c} +} + +// ListExecution provides a mock function with given fields: ctx, project, domain, filter +func (_m *AdminFetcherExtInterface) ListExecution(ctx context.Context, project string, domain string, filter filters.Filters) (*admin.ExecutionList, error) { + ret := _m.Called(ctx, project, domain, filter) + + var r0 *admin.ExecutionList + if rf, ok := ret.Get(0).(func(context.Context, string, string, filters.Filters) *admin.ExecutionList); ok { + r0 = rf(ctx, project, domain, filter) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*admin.ExecutionList) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func(context.Context, string, string, filters.Filters) error); ok { + r1 = rf(ctx, project, domain, filter) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +type AdminFetcherExtInterface_ListProjects struct { + *mock.Call +} + +func (_m AdminFetcherExtInterface_ListProjects) Return(_a0 *admin.Projects, _a1 error) *AdminFetcherExtInterface_ListProjects { + return &AdminFetcherExtInterface_ListProjects{Call: _m.Call.Return(_a0, _a1)} +} + +func (_m *AdminFetcherExtInterface) OnListProjects(ctx context.Context, filter filters.Filters) *AdminFetcherExtInterface_ListProjects { + c := _m.On("ListProjects", ctx, filter) + return &AdminFetcherExtInterface_ListProjects{Call: c} +} + +func (_m *AdminFetcherExtInterface) OnListProjectsMatch(matchers ...interface{}) *AdminFetcherExtInterface_ListProjects { + c := _m.On("ListProjects", matchers...) + return &AdminFetcherExtInterface_ListProjects{Call: c} +} + +// ListProjects provides a mock function with given fields: ctx, filter +func (_m *AdminFetcherExtInterface) ListProjects(ctx context.Context, filter filters.Filters) (*admin.Projects, error) { + ret := _m.Called(ctx, filter) + + var r0 *admin.Projects + if rf, ok := ret.Get(0).(func(context.Context, filters.Filters) *admin.Projects); ok { + r0 = rf(ctx, filter) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*admin.Projects) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func(context.Context, filters.Filters) error); ok { + r1 = rf(ctx, filter) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} diff --git a/pkg/ext/project_fetcher.go b/pkg/ext/project_fetcher.go new file mode 100644 index 0000000000..4367bef9a6 --- /dev/null +++ b/pkg/ext/project_fetcher.go @@ -0,0 +1,21 @@ +package ext + +import ( + "context" + + "github.com/flyteorg/flytectl/pkg/filters" + + "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin" +) + +func (a *AdminFetcherExtClient) ListProjects(ctx context.Context, filter filters.Filters) (*admin.Projects, error) { + transformFilters, err := filters.BuildProjectListRequest(filter) + if err != nil { + return nil, err + } + e, err := a.AdminServiceClient().ListProjects(ctx, transformFilters) + if err != nil { + return nil, err + } + return e, nil +} diff --git a/pkg/ext/project_fetcher_test.go b/pkg/ext/project_fetcher_test.go new file mode 100644 index 0000000000..4b9c505485 --- /dev/null +++ b/pkg/ext/project_fetcher_test.go @@ -0,0 +1,45 @@ +package ext + +import ( + "testing" + + "github.com/flyteorg/flyteidl/clients/go/admin/mocks" + "github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" +) + +func TestAdminFetcherExtClient_ListProjects(t *testing.T) { + + project1 := &admin.Project{ + Id: "flyteexample", + Name: "flyteexample", + Domains: []*admin.Domain{ + { + Id: "development", + Name: "development", + }, + }, + } + + project2 := &admin.Project{ + Id: "flytesnacks", + Name: "flytesnacks", + Domains: []*admin.Domain{ + { + Id: "development", + Name: "development", + }, + }, + } + + adminClient = new(mocks.AdminServiceClient) + adminFetcherExt = AdminFetcherExtClient{AdminClient: adminClient} + + projects := &admin.Projects{ + Projects: []*admin.Project{project1, project2}, + } + adminClient.OnListProjectsMatch(mock.Anything, mock.Anything).Return(projects, nil) + _, err := adminFetcherExt.ListProjects(ctx, taskFilter) + assert.Nil(t, err) +}