Single Activity API changes #534

ychebotarev · 2025-01-28T00:43:51Z

What changed?
This PR contains a commutative feedback for single activity API:

add 'jitter' fields to Reset and Unpause
'Flatten' unpause. Previous approach was marked as overcomplicated
Add 'keep_unpause' flag to Reset. By default reset will also unpause the activity.
Remove no-wait flag. By default, if activity is in retry, they will be scheduled immediately (*jitter)
Add "activity type" as a routing parameter to every activity operation. If activity type is provided - every pending activity of this type will be paused/reset/updated/unpaused
Update API descriptions to reflect those changes.

Why?
Working on single activity API feedback from the design review.

Breaking changes
Yes

Server PR
Add batch activity unpause. Single activity commulative changes

cretz · 2025-01-28T16:03:13Z

temporal/api/workflowservice/v1/request_response.proto

@@ -1767,8 +1765,18 @@ message UpdateActivityOptionsByIdRequest {
    // Controls which fields from `activity_options` will be applied
    google.protobuf.FieldMask update_mask = 7;

-    // Used to de-dupe requests
-    string request_id = 8;
+    // either activity id or activity type must be provided


or activity type

This doesn't make sense in an UpdateActivityOptionsByIdRequest IMO. This API call as named only makes sense for a single activity by ID. If there are batch needs or a non-ID-specific update, maybe a different API call is needed or this one is improperly named.

Being able to provide activity type was part of the feedback. If you have multiple activities of the same type running in parallel (the most common scenario) - you want to pause all of them. Doing this 1 by 1 providing activity ids is tiresome.

You right, now "ByID" make less sense.
As I remember, I added "ByID" because of other "ByID" functions that has the same routings (as opposite to "task token")

I'm totally ok with removing it. It will be a breaking change though.

I also will not call it "batch", it still a single workflow, and we use term "batch" for multiple workflows.

since we are doing this - I also change "workflowID+runID" to execution_info, like in every other API.

looks much cleaner now

temporal/api/workflowservice/v1/request_response.proto

temporal/api/workflowservice/v1/service.proto

Sushisource · 2025-01-28T19:34:22Z

temporal/api/workflowservice/v1/request_response.proto

    // Activity options after an update
    temporal.api.activity.v1.ActivityOptions activity_options = 1;
 }

-message PauseActivityByIdRequest {
+message PauseActivityRequest {


Will we be adding a should_interrupt or no_interrupt flag? Or is that coming later? Not blocking this PR by any means.

I personally think we shouldn't add anything at all, just come with a good default. If users want to "pause if it fails" then can add this as a "rule".
That said - if we decide to add this kind of flag then yes, it will be in this request.

temporal/api/workflowservice/v1/request_response.proto

alexshtin

Yes, this looks much better now.

cretz · 2025-01-29T15:48:28Z

temporal/api/workflowservice/v1/service.proto

-    // UpdateActivityOptionsById is called by the client to update the options of an activity
-    // (-- api-linter: core::0136::prepositions=disabled
-    //     aip.dev/not-precedent: "By" is used to indicate request type. --)
-    rpc UpdateActivityOptionsById (UpdateActivityOptionsByIdRequest) returns (UpdateActivityOptionsByIdResponse) {


While I understand this is not GA, this is a backwards incompatible alteration of a released API, which means it is consumed by proxies. Also, even though it is not GA, it is in the CLI and this incompatible alteration means that the current version of CLI cannot be used for this feature even after the feature is GA. It also means, once tagged, we cannot upgrade the API version in the CLI project (which we may want to do for other reasons) because compile will break until the code is altered to match the new API, and then that may not work with the server.

This is unfortunate but technically doable. As mentioned when these APIs were first getting to master, this is the cost of not getting further along on implementation.

I would like to request we wait to land this in master until we have most of the implementation done (maybe it already is), we are ready to tag the API, and we have a PR ready for the CLI to support these incompatible alterations. Then we can merge this and tag the API and update CLI (ignoring that it may not even work with the latest server).

this is the cost of not getting further along on implementation

we fully implement those API before adding it to CLI.

we wait to land this in master until we have most of the implementation done

All (re)implementation is done - see the link to the Server PR in description. Of cause it needs to be changed based on this PR.

cretz · 2025-01-29T15:51:32Z

I think we should investigate why our buf-breaking didn't cause a CI failure here

Sushisource

A few more grammar fixes.

I agree with Chad about not merging until we're satisfied with the implementation

Corresponding server PR is in the description.

temporal/api/workflowservice/v1/request_response.proto

temporal/api/workflowservice/v1/service.proto

ychebotarev requested review from a team as code owners January 28, 2025 00:43

ychebotarev assigned alexshtin Jan 28, 2025

cretz reviewed Jan 28, 2025

View reviewed changes

ychebotarev requested a review from cretz January 28, 2025 19:10

Sushisource reviewed Jan 28, 2025

View reviewed changes

ychebotarev requested a review from Sushisource January 28, 2025 20:17

alexshtin reviewed Jan 28, 2025

View reviewed changes

temporal/api/workflowservice/v1/request_response.proto Outdated Show resolved Hide resolved

alexshtin reviewed Jan 28, 2025

View reviewed changes

ychebotarev requested a review from alexshtin January 28, 2025 23:52

alexshtin approved these changes Jan 29, 2025

View reviewed changes

cretz reviewed Jan 29, 2025

View reviewed changes

ychebotarev requested a review from cretz January 29, 2025 17:18

Sushisource reviewed Jan 29, 2025

View reviewed changes

ychebotarev requested a review from Sushisource January 29, 2025 18:18

Sushisource approved these changes Jan 29, 2025

View reviewed changes

ychebotarev added 5 commits January 30, 2025 09:45

Single Activity API changes

904cf6b

remove ByID from acitvity API. Clean the code

f339106

Work on Spencer comments

6e9b5a6

comments

873f066

add unpause all to match batch unpause

7e60c7c

ychebotarev force-pushed the y_final branch from 597db69 to 7e60c7c Compare January 30, 2025 17:45

ychebotarev merged commit a1edc73 into master Jan 30, 2025
4 checks passed

ychebotarev deleted the y_final branch January 30, 2025 17:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Single Activity API changes #534

Single Activity API changes #534

ychebotarev commented Jan 28, 2025 •

edited

Loading

cretz Jan 28, 2025

ychebotarev Jan 28, 2025 •

edited

Loading

ychebotarev Jan 28, 2025

ychebotarev Jan 28, 2025

Sushisource Jan 28, 2025

ychebotarev Jan 28, 2025 •

edited

Loading

alexshtin left a comment

cretz Jan 29, 2025

ychebotarev Jan 29, 2025 •

edited

Loading

cretz commented Jan 29, 2025

Sushisource left a comment •

edited by ychebotarev

Loading

Single Activity API changes #534

Single Activity API changes #534

Conversation

ychebotarev commented Jan 28, 2025 • edited Loading

cretz Jan 28, 2025

Choose a reason for hiding this comment

ychebotarev Jan 28, 2025 • edited Loading

Choose a reason for hiding this comment

ychebotarev Jan 28, 2025

Choose a reason for hiding this comment

ychebotarev Jan 28, 2025

Choose a reason for hiding this comment

Sushisource Jan 28, 2025

Choose a reason for hiding this comment

ychebotarev Jan 28, 2025 • edited Loading

Choose a reason for hiding this comment

alexshtin left a comment

Choose a reason for hiding this comment

cretz Jan 29, 2025

Choose a reason for hiding this comment

ychebotarev Jan 29, 2025 • edited Loading

Choose a reason for hiding this comment

cretz commented Jan 29, 2025

Sushisource left a comment • edited by ychebotarev Loading

Choose a reason for hiding this comment

ychebotarev commented Jan 28, 2025 •

edited

Loading

ychebotarev Jan 28, 2025 •

edited

Loading

ychebotarev Jan 28, 2025 •

edited

Loading

ychebotarev Jan 29, 2025 •

edited

Loading

Sushisource left a comment •

edited by ychebotarev

Loading