From 164508f2342fd7d115dc88429163f0daab0149e1 Mon Sep 17 00:00:00 2001 From: Aditi Gautam Date: Thu, 25 Jul 2024 10:44:00 -0700 Subject: [PATCH 1/2] Added minor adjustments in the shadow status options --- internal/query_builder.go | 4 +--- internal/query_builder_test.go | 8 +------- internal/workflow_shadower.go | 5 ++--- internal/workflow_shadower_test.go | 4 ++-- 4 files changed, 6 insertions(+), 15 deletions(-) diff --git a/internal/query_builder.go b/internal/query_builder.go index 9519a0c32..e50b1a9ab 100644 --- a/internal/query_builder.go +++ b/internal/query_builder.go @@ -50,8 +50,6 @@ var ( WorkflowStatusFailed = WorkflowStatus(shared.WorkflowExecutionCloseStatusFailed.String()) // WorkflowStatusCanceled is the WorkflowStatus for canceled workflows WorkflowStatusCanceled = WorkflowStatus(shared.WorkflowExecutionCloseStatusCanceled.String()) - // WorkflowStatusTerminated is the WorkflowStatus for terminated workflows - WorkflowStatusTerminated = WorkflowStatus(shared.WorkflowExecutionCloseStatusTerminated.String()) // WorkflowStatusContinuedAsNew is the WorkflowStatus for continuedAsNew workflows WorkflowStatusContinuedAsNew = WorkflowStatus(shared.WorkflowExecutionCloseStatusContinuedAsNew.String()) // WorkflowStatusTimedOut is the WorkflowStatus for timedout workflows @@ -183,7 +181,7 @@ func ToWorkflowStatus(statusString string) (WorkflowStatus, error) { status := WorkflowStatus(strings.ToUpper(statusString)) switch status { case WorkflowStatusOpen, WorkflowStatusClosed, WorkflowStatusCompleted, - WorkflowStatusFailed, WorkflowStatusCanceled, WorkflowStatusTerminated, + WorkflowStatusFailed, WorkflowStatusCanceled, WorkflowStatusContinuedAsNew, WorkflowStatusTimedOut, WorkflowStatusALL: return status, nil default: diff --git a/internal/query_builder_test.go b/internal/query_builder_test.go index 1b005249c..310520f8e 100644 --- a/internal/query_builder_test.go +++ b/internal/query_builder_test.go @@ -103,7 +103,7 @@ func (s *queryBuilderSuite) TestWorkflowStatusQuery() { }, { msg: "all workflows", - workflowStatuses: []WorkflowStatus{WorkflowStatusTerminated, WorkflowStatusALL}, + workflowStatuses: []WorkflowStatus{WorkflowStatusALL}, expectedQuery: "", }, } @@ -225,12 +225,6 @@ func (s *queryBuilderSuite) TestToWorkflowStatus() { expectErr: false, expectedStatus: WorkflowStatusTimedOut, }, - { - msg: "upper case status string", - statusString: "TERMINATED", - expectErr: false, - expectedStatus: WorkflowStatusTerminated, - }, { msg: "all", statusString: "ALL", diff --git a/internal/workflow_shadower.go b/internal/workflow_shadower.go index 19db99897..059885768 100644 --- a/internal/workflow_shadower.go +++ b/internal/workflow_shadower.go @@ -338,10 +338,9 @@ func (o *ShadowOptions) validateAndPopulateFields() error { } statuses = append(statuses, status) } - //All the open statuses are taken by default. This list seems to not work as expected. - //TODO: verify that the status list works as expected. currently all wfs of all types get picked up. + //This filter doesn't seem to be working as expected. if len(statuses) == 0 { - statuses = []WorkflowStatus{WorkflowStatusOpen} + statuses = []WorkflowStatus{WorkflowStatusOpen, WorkflowStatusClosed} } queryBuilder.WorkflowStatus(statuses) diff --git a/internal/workflow_shadower_test.go b/internal/workflow_shadower_test.go index edf7fef85..c89232b5c 100644 --- a/internal/workflow_shadower_test.go +++ b/internal/workflow_shadower_test.go @@ -203,7 +203,7 @@ func (s *workflowShadowerSuite) TestShadowOptionsValidation() { options: ShadowOptions{}, expectErr: false, validationFn: func(options *ShadowOptions) { - s.Equal("(CloseTime = missing)", options.WorkflowQuery) + s.Equal("(CloseTime = missing or CloseTime != missing)", options.WorkflowQuery) s.Equal(1.0, options.SamplingRate) s.Equal(1, options.Concurrency) }, @@ -255,7 +255,7 @@ func (s *workflowShadowerSuite) TestShadowOptionsWithExcludeTypes() { Mode: ShadowModeNormal, } expectedQuery := fmt.Sprintf( - `(WorkflowType = "includedType1" or WorkflowType = "includedType2") and (WorkflowType != "excludedType1" and WorkflowType != "excludedType2") and (CloseTime = missing)`, + `(WorkflowType = "includedType1" or WorkflowType = "includedType2") and (WorkflowType != "excludedType1" and WorkflowType != "excludedType2") and (CloseTime = missing or CloseTime != missing)`, ) shadower, err := NewWorkflowShadower(s.mockService, "testDomain", options, ReplayOptions{}, nil) s.NoError(err) From 3f2c2fc61b45d6bbf3148cf220ce2be3d21fe875 Mon Sep 17 00:00:00 2001 From: Aditi Gautam Date: Tue, 30 Jul 2024 10:20:00 -0700 Subject: [PATCH 2/2] Mini fix --- internal/workflow_shadower.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/internal/workflow_shadower.go b/internal/workflow_shadower.go index 059885768..93a94673b 100644 --- a/internal/workflow_shadower.go +++ b/internal/workflow_shadower.go @@ -338,10 +338,13 @@ func (o *ShadowOptions) validateAndPopulateFields() error { } statuses = append(statuses, status) } - //This filter doesn't seem to be working as expected. + // TODO: handle ContinueAsNew as a special case. + // This filter doesn't seem to be working as expected. + // This filter should have only taken open and closed wfs but it returns continue as new wfs as well. if len(statuses) == 0 { statuses = []WorkflowStatus{WorkflowStatusOpen, WorkflowStatusClosed} } + queryBuilder.WorkflowStatus(statuses) if !o.WorkflowStartTimeFilter.isEmpty() {