Add fields for batch request/response #216
Conversation
| string reason = 3; | ||
| // Operator of the operation | ||
| string operator = 4; |
| // Operator indicates the batch operation starter identity | ||
| string operator = 9; |
There was a problem hiding this comment.
This might be confusing. The batch operation start request has the identity field, which as explained is the identity of the worker/client, is this operator the same as that identity?
| @@ -1111,6 +1117,8 @@ message DescribeBatchOperationResponse { | |||
| int64 complete_operation_count = 7; | |||
| // Failure operation count | |||
| int64 failure_operation_count = 8; | |||
| // Identity indicates the operator identity | |||
| string identity = 9; | |||
There was a problem hiding this comment.
I'd put the reason in the response too
| @@ -1064,6 +1064,8 @@ message StartBatchOperationRequest { | |||
| string namespace = 1; | |||
| // Visibility query defines the the group of workflow to do batch operation | |||
| string visibility_query = 2; | |||
| // Job ID defines the unique ID for the batch job | |||
| string job_id = 3; | |||
There was a problem hiding this comment.
Is this required? If so, why do we need it in the response? It's a bit confusing IMHO.
There was a problem hiding this comment.
Should this include a reason too? I know terminate and cancel batch operations include a reason but signal doesn't.
There was a problem hiding this comment.
Had a discussion. To avoid duplication, we allow pass in job id. And I will remove the job_id from response. The reason is included in the operation options.
There was a problem hiding this comment.
Yes. There are two options termination, cancellation, signal. The first two options contain reason.
There was a problem hiding this comment.
I'd consider adding a reason here and remove it from the batch operations so there can be a reason for batch signal too.
| reserved 1; | ||
| reserved "job_id"; |
There was a problem hiding this comment.
Since this was never released it's okay to delete at this point without reserving the field.
What changed?
Add fields for batch request/response
Why?
Breaking changes