Add schedule rpc handlers #2857
Conversation
| } | ||
|
|
||
| // Returns the schedule description and current state of an existing schedule. | ||
| func (wh *WorkflowHandler) DescribeSchedule(ctx context.Context, request *workflowservice.DescribeScheduleRequest) (_ *workflowservice.DescribeScheduleResponse, retError error) { | ||
| return nil, serviceerror.NewUnimplemented("todo") | ||
| defer log.CapturePanic(wh.logger, &retError) |
There was a problem hiding this comment.
any idea what is typical latency for DescribeSchedule?
There was a problem hiding this comment.
in local testing it's been "fast enough". when it has to do a refresh it is slightly noticeable to me compared to the no-refresh case, so I'm guessing maybe 50-100ms more?
service/frontend/workflowHandler.go
Outdated
| // TODO: should we respect this here? | ||
| if wh.config.DisableListVisibilityByFilter(namespaceName.String()) { | ||
| return nil, errNoPermission | ||
| } |
There was a problem hiding this comment.
hmm, i think the reason we have that dynamic config is because the cassandra secondary index would bring down cass cluster if the data is too big. We probably should honor that setting. But maybe we should have clear error message or error logging to indicate that setting is the reason the request is failing. Otherwise, people won't be able to find out why it is failing.
Later, we should really get rid of cassandra visibility store.
There was a problem hiding this comment.
I changed the error message to be more specific
service/frontend/workflowHandler.go
Outdated
| // TODO: is this correct? where does search attribute mapping happen? | ||
| SearchAttributes: ex.GetSearchAttributes(), |
There was a problem hiding this comment.
turns out it happens inside the ES visibility manager, and the other visibility stores don't support search attributes so they don't do it
There was a problem hiding this comment.
which is not a good example of responsibility separation and needs to be changed one day.
What changed?
Add handlers for the frontend schedule-related rpcs.
Why?
Implementation of new scheduled workflow functionality.
How did you test it?
Manual testing, will write integration tests next.
Potential risks
Is hotfix candidate?