Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove query consistency level from request #452

Merged

Conversation

alexshtin
Copy link
Member

What changed?
QueryConsistencyLevel field was removed from QueryWorkflow request and dynamic config with default value STRONG.

Why?
EVENTUAL consistency is not supported any more.

How did you test it?
Run all tests.

Potential risks
No tests.

if request.GetRequest().GetQueryConsistencyLevel() == enumspb.QUERY_CONSISTENCY_LEVEL_STRONG && !consistentQueryEnabled {
return nil, ErrConsistentQueryNotEnabled
}
queryConsistencyLevel := enumspb.QUERY_CONSISTENCY_LEVEL_STRONG
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left this as never change local var here in case if we decide to bring it back one day.

},
}
resp, err := s.mockHistoryEngine.QueryWorkflow(context.Background(), request)
s.Equal(ErrQueryWorkflowBeforeFirstDecision, err)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This error was returned only from eventual consistency block.

Comment on lines -655 to -656
di = addDecisionTaskScheduledEvent(msBuilder)
addDecisionTaskStartedEvent(msBuilder, di.ScheduleID, tasklist, identity)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only completed workflows can be queried directly through matching now.

@alexshtin alexshtin merged commit 8786f00 into temporalio:master Jun 14, 2020
@alexshtin alexshtin deleted the feature/remove-query-consistency-level branch June 14, 2020 01:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants