Versioning 3 #1744
Versioning 3 #1744
Conversation
* Rename DeploymentName --> DeploymentSeriesName
|
Overall the client API makes sense to me, I didn't review the underlying gRPC api to ensure the API is being invoked or mocked correctly. |
| func (iter *deploymentListIteratorImpl) HasNext() bool { | ||
| if iter.err == nil && | ||
| (iter.response == nil || | ||
| (iter.nextDeploymentIndex >= len(iter.response.Deployments) && len(iter.response.NextPageToken) > 0)) { |
There was a problem hiding this comment.
ListDeployments uses visibility.ListWorkflowExecutions under the hood directly, the same way that ListSchedules does, so the contents of a page and the nextPageToken are handled the same for ListDeployments and ListSchedules. Because of this, I believe that there is no risk of an empty page followed by a non-empty page.
There was a problem hiding this comment.
In the elastic search visibility store implementation, next page token is only non-nil if the number of entries on the page == pageSize: https://github.com/temporalio/temporal/blob/main/common/persistence/visibility/store/elasticsearch/visibility_store.go#L839-L848
There was a problem hiding this comment.
@cretz @antlai-temporal hope this helps
There was a problem hiding this comment.
same in the sql visibility store implementation, next page token is only non-nil if the number of entries on the page == pageSize: https://github.com/temporalio/temporal/blob/main/common/persistence/visibility/store/sql/visibility_store.go#L233-L248
There was a problem hiding this comment.
I believe that there is no risk of an empty page followed by a non-empty page
Sounds good. Just wanted to make sure there was no post-visibility item filtering done server side that would affect this.
|
@cretz Integration tests against the new cli ok... |
What was changed
This PR implements a new deployment based approach for Worker Versioning in go.
This is an umbrella PR, with several PRs in the versioning-3 branch already reviewed:
#1675
#1692
#1708
#1722
#1727
#1733
#1742
It is marked as draft, missing some system tests, and waiting on a CLI release that provides the new server.
But even in draft mode, it should be reasonably stable, please give feedback... Thanks!