add more states to pending activift info #457
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -106,6 +106,8 @@ enum PendingActivityState { | |
| PENDING_ACTIVITY_STATE_SCHEDULED = 1; | ||
| PENDING_ACTIVITY_STATE_STARTED = 2; | ||
| PENDING_ACTIVITY_STATE_CANCEL_REQUESTED = 3; | ||
| PENDING_ACTIVITY_STATE_BACKING_OFF = 4; | ||
| PENDING_ACTIVITY_STATE_PAUSED = 5; | ||
|
There was a problem hiding this comment. As discussed in another PR. I wouldn't model this as a distinct state and instead have another dimension for expressing pause status to be able to represent scenarios such as: an attempt is currently started and the activity has been requested to pause. There was a problem hiding this comment. From user perspective I don't see a scenario when it matters from what state activity comes to the PAUSE state. There was a problem hiding this comment. Agreed. These states are more where the activity is at in general (started, scheduled, etc) and not specific things about the already-started activity. Would like to see more specific details on the pending activity itself (and really, the describe activity and pending activity should have the same information, ideally even the same object). There was a problem hiding this comment. I'm not 100% sure what you agree with @cretz. api/temporal/api/schedule/v1/message.proto Line 281 in c038641 I would rather have this consistency. There was a problem hiding this comment. Sorry, I see my message posted at the same time as @ychebotarev. I agree with the original statement that should not be in the enumerate for activity state. A paused or backing off activity is still "scheduled" or "started" based on how this enum is used. Not only is it a backwards incompatible change to suddenly start making backing-off activities be a different enum, but I think whether an activity is paused or backing-off is unrelated to this state which is more of an overall one of whether it has ever started. There was a problem hiding this comment. @cretz - there will be bool variable in PendingActivityInfo that indicates that activity is paused. PendingActivityInfo will also be part of the DescribeActivity response. This state is a separate thing. It is more about "give it quick look and understand where are we". There was a problem hiding this comment. "started" is not "running" and to make people's code that expects backing-off activities to be this new enum instead of what it always was is backwards incompatible. If we need a more detailed value somewhere because "started" needs to be split into multiple values, that makes sense. IMO a paused activity can be scheduled/started, as can a backing off activity (and is how people have coded against it today). |
||
| } | ||
|
|
||
| enum PendingWorkflowTaskState { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As mentioned in our slack conversation. This is an artificial state and the state machine doesn't record this explicitly to save DB updates. I think it's nice to be able to surface this state to users though so I support adding it to the API.