Skip to content

Commit

Permalink
chore(run): resolve comments
Browse files Browse the repository at this point in the history
  • Loading branch information
joremysh committed Oct 16, 2024
1 parent d0ba661 commit 7ff990d
Show file tree
Hide file tree
Showing 11 changed files with 310 additions and 307 deletions.
27 changes: 5 additions & 22 deletions pkg/handler/pipeline.go
Original file line number Diff line number Diff line change
Expand Up @@ -1995,36 +1995,19 @@ func (h *PublicHandler) ListComponentRuns(ctx context.Context, req *pb.ListCompo
return resp, nil
}

// todo: rename function to ListPipelineRunsByRequester in protobuf and update message names here
func (h *PublicHandler) ListPipelineRunsByCreditOwner(ctx context.Context, req *pb.ListPipelineRunsByCreditOwnerRequest) (*pb.ListPipelineRunsByCreditOwnerResponse, error) {
logger, _ := logger.GetZapLogger(ctx)
logUUID, _ := uuid.NewV4()
logger.Info("ListPipelineRunsByCreditOwner starts", zap.String("logUUID", logUUID.String()))
logger.Info("ListPipelineRunsByRequester starts", zap.String("logUUID", logUUID.String()))

if req.GetStart().IsValid() && req.GetStop().IsValid() && req.GetStart().AsTime().After(req.GetStop().AsTime()) {
return nil, fmt.Errorf("input stop time earlier than start time")
}

declarations, err := filtering.NewDeclarations([]filtering.DeclarationOption{
filtering.DeclareStandardFunctions(),
filtering.DeclareIdent("status", filtering.TypeString),
filtering.DeclareIdent("source", filtering.TypeString),
}...)
if err != nil {
return nil, err
}

filter, err := filtering.ParseFilter(req, declarations)
if err != nil {
return nil, err
}

resp, err := h.service.ListPipelineRunsByCreditOwner(ctx, req, filter)
resp, err := h.service.ListPipelineRunsByRequester(ctx, req)
if err != nil {
logger.Error("failed in ListPipelineRunsByCreditOwner", zap.String("logUUID", logUUID.String()), zap.Error(err))
logger.Error("failed in ListPipelineRunsByRequester", zap.String("logUUID", logUUID.String()), zap.Error(err))
return nil, status.Error(codes.Internal, "Failed to list pipeline runs")
}

logger.Info("ListPipelineRunsByCreditOwner finished", zap.String("logUUID", logUUID.String()))
logger.Info("ListPipelineRunsByRequester finished", zap.String("logUUID", logUUID.String()))

return resp, nil
}
2 changes: 2 additions & 0 deletions pkg/mock/acl_client_interface_mock.gen.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions pkg/mock/converter_mock.gen.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 5 additions & 2 deletions pkg/mock/mgmt_private_service_client_mock.gen.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 4 additions & 1 deletion pkg/mock/minio_i_mock.gen.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

535 changes: 268 additions & 267 deletions pkg/mock/repository_mock.gen.go

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions pkg/repository/repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ type Repository interface {

GetPaginatedPipelineRunsWithPermissions(ctx context.Context, requesterUID, pipelineUID string, page, pageSize int, filter filtering.Filter, order ordering.OrderBy, isOwner bool) ([]datamodel.PipelineRun, int64, error)
GetPaginatedComponentRunsByPipelineRunIDWithPermissions(ctx context.Context, pipelineRunID string, page, pageSize int, filter filtering.Filter, order ordering.OrderBy) ([]datamodel.ComponentRun, int64, error)
GetPaginatedPipelineRunsByCreditOwner(ctx context.Context, requesterUID string, startTime, endTime time.Time, page, pageSize int, filter filtering.Filter, order ordering.OrderBy) ([]datamodel.PipelineRun, int64, error)
GetPaginatedPipelineRunsByRequester(ctx context.Context, requesterUID string, startTime, endTime time.Time, page, pageSize int, filter filtering.Filter, order ordering.OrderBy) ([]datamodel.PipelineRun, int64, error)
}

type repository struct {
Expand Down Expand Up @@ -1270,7 +1270,7 @@ func (r *repository) GetPaginatedComponentRunsByPipelineRunIDWithPermissions(ctx
return componentRuns, totalRows, nil
}

func (r *repository) GetPaginatedPipelineRunsByCreditOwner(ctx context.Context, requesterUID string, startTimeBegin, startTimeEnd time.Time, page, pageSize int, filter filtering.Filter, order ordering.OrderBy) ([]datamodel.PipelineRun, int64, error) {
func (r *repository) GetPaginatedPipelineRunsByRequester(ctx context.Context, requesterUID string, startTimeBegin, startTimeEnd time.Time, page, pageSize int, filter filtering.Filter, order ordering.OrderBy) ([]datamodel.PipelineRun, int64, error) {
var pipelineRuns []datamodel.PipelineRun
var totalRows int64

Expand Down
8 changes: 3 additions & 5 deletions pkg/repository/repository_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -613,8 +613,6 @@ func TestRepository_GetPaginatedPipelineRunsWithPermissions(t *testing.T) {
c.Assert(err, qt.IsNil)
if testCase.canView {
c.Check(len(response), qt.Equals, 1)
// c.Log(response[0].Pipeline.ID)
// c.Check(response[0].Pipeline.ID, qt.Equals, p.ID)
} else {
c.Check(len(response), qt.Equals, 0)
}
Expand Down Expand Up @@ -698,11 +696,11 @@ func TestRepository_GetPaginatedPipelineRunsByCreditOwner(t *testing.T) {
err = repo.UpsertPipelineRun(ctx, pipelineRun)
require.NoError(t, err)

resp, _, err := repo.GetPaginatedPipelineRunsByCreditOwner(ctx, namespace1, now.Add(-3*time.Hour), now.Add(-2*time.Hour), 0, 10, filtering.Filter{}, ordering.OrderBy{})
resp, _, err := repo.GetPaginatedPipelineRunsByRequester(ctx, namespace1, now.Add(-3*time.Hour), now.Add(-2*time.Hour), 0, 10, filtering.Filter{}, ordering.OrderBy{})
require.NoError(t, err)
require.Len(t, resp, 0)

resp, _, err = repo.GetPaginatedPipelineRunsByCreditOwner(ctx, namespace1, now.Add(-2*time.Hour), now, 0, 10, filtering.Filter{}, ordering.OrderBy{})
resp, _, err = repo.GetPaginatedPipelineRunsByRequester(ctx, namespace1, now.Add(-2*time.Hour), now, 0, 10, filtering.Filter{}, ordering.OrderBy{})
require.NoError(t, err)
require.Len(t, resp, 1)
require.Equal(t, resp[0].PipelineTriggerUID, pipelineRun.PipelineTriggerUID)
Expand All @@ -723,7 +721,7 @@ func TestRepository_GetPaginatedPipelineRunsByCreditOwner(t *testing.T) {
err = repo.UpsertPipelineRun(ctx, pipelineRun2)
require.NoError(t, err)

resp, _, err = repo.GetPaginatedPipelineRunsByCreditOwner(ctx, namespace1, now.Add(-2*time.Hour), now, 0, 10, filtering.Filter{}, ordering.OrderBy{})
resp, _, err = repo.GetPaginatedPipelineRunsByRequester(ctx, namespace1, now.Add(-2*time.Hour), now, 0, 10, filtering.Filter{}, ordering.OrderBy{})
require.NoError(t, err)
require.Len(t, resp, 2)
require.Equal(t, resp[0].PipelineTriggerUID, pipelineRun.PipelineTriggerUID)
Expand Down
4 changes: 1 addition & 3 deletions pkg/service/component_definition.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,10 @@ import (
"github.com/instill-ai/pipeline-backend/config"
"github.com/instill-ai/pipeline-backend/pkg/recipe"
"github.com/instill-ai/pipeline-backend/pkg/repository"
"github.com/instill-ai/x/paginate"

component "github.com/instill-ai/pipeline-backend/pkg/component/store"
errdomain "github.com/instill-ai/pipeline-backend/pkg/errors"

"github.com/instill-ai/x/paginate"

pb "github.com/instill-ai/protogen-go/vdp/pipeline/v1beta"
)

Expand Down
2 changes: 1 addition & 1 deletion pkg/service/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ type Service interface {

ListPipelineRuns(ctx context.Context, req *pb.ListPipelineRunsRequest, filter filtering.Filter) (*pb.ListPipelineRunsResponse, error)
ListComponentRuns(ctx context.Context, req *pb.ListComponentRunsRequest, filter filtering.Filter) (*pb.ListComponentRunsResponse, error)
ListPipelineRunsByCreditOwner(ctx context.Context, req *pb.ListPipelineRunsByCreditOwnerRequest, filter filtering.Filter) (*pb.ListPipelineRunsByCreditOwnerResponse, error)
ListPipelineRunsByRequester(ctx context.Context, req *pb.ListPipelineRunsByCreditOwnerRequest) (*pb.ListPipelineRunsByCreditOwnerResponse, error)

GetIntegration(_ context.Context, id string, _ pb.View) (*pb.Integration, error)
ListIntegrations(context.Context, *pb.ListIntegrationsRequest) (*pb.ListIntegrationsResponse, error)
Expand Down
20 changes: 16 additions & 4 deletions pkg/service/pipeline.go
Original file line number Diff line number Diff line change
Expand Up @@ -1992,14 +1992,27 @@ func (s *service) ListComponentRuns(ctx context.Context, req *pipelinepb.ListCom
}, nil
}

func (s *service) ListPipelineRunsByCreditOwner(ctx context.Context, req *pipelinepb.ListPipelineRunsByCreditOwnerRequest,
filter filtering.Filter) (*pipelinepb.ListPipelineRunsByCreditOwnerResponse, error) {
func (s *service) ListPipelineRunsByRequester(ctx context.Context, req *pipelinepb.ListPipelineRunsByCreditOwnerRequest) (*pipelinepb.ListPipelineRunsByCreditOwnerResponse, error) {
page := s.pageInRange(req.GetPage())
pageSize := s.pageSizeInRange(req.GetPageSize())
requesterUID, _ := utils.GetRequesterUIDAndUserUID(ctx)

log, _ := logger.GetZapLogger(ctx)

declarations, err := filtering.NewDeclarations([]filtering.DeclarationOption{
filtering.DeclareStandardFunctions(),
filtering.DeclareIdent("status", filtering.TypeString),
filtering.DeclareIdent("source", filtering.TypeString),
}...)
if err != nil {
return nil, err
}

filter, err := filtering.ParseFilter(req, declarations)
if err != nil {
return nil, err
}

orderBy, err := ordering.ParseOrderBy(req)
if err != nil {
return nil, err
Expand All @@ -2018,9 +2031,8 @@ func (s *service) ListPipelineRunsByCreditOwner(ctx context.Context, req *pipeli
if startedTimeBegin.After(startedTimeEnd) {
return nil, fmt.Errorf("time range end time %s is earlier than start time %s", startedTimeEnd.Format(time.RFC3339), startedTimeBegin.Format(time.RFC3339))
}
log.Info("ListPipelineRunsByCreditOwner", zap.Time("startedTimeBegin", startedTimeBegin), zap.Time("startedTimeEnd", startedTimeEnd))

pipelineRuns, totalCount, err := s.repository.GetPaginatedPipelineRunsByCreditOwner(ctx, requesterUID, startedTimeBegin, startedTimeEnd,
pipelineRuns, totalCount, err := s.repository.GetPaginatedPipelineRunsByRequester(ctx, requesterUID, startedTimeBegin, startedTimeEnd,
page, pageSize, filter, orderBy)
if err != nil {
return nil, fmt.Errorf("failed to get pipeline runs: %w", err)
Expand Down

0 comments on commit 7ff990d

Please sign in to comment.