Add DescribeActivityById definition #462
Conversation
| // Activity was requested to be canceled. | ||
| ACTIVITY_STATE_CANCEL_REQUESTED = 3; | ||
| // Activity failed and is backing off before retrying. | ||
| ACTIVITY_STATE_CANCEL_BACKING_OFF = 4; |
There was a problem hiding this comment.
| ACTIVITY_STATE_CANCEL_BACKING_OFF = 4; | |
| ACTIVITY_STATE_BACKING_OFF = 4; |
There was a problem hiding this comment.
Please don't resolve comments. I keep seeing my suggestions disappear on PRs and not always being addressed or commented on.
There was a problem hiding this comment.
tolked offline. I'l be resolving only after I push new version
| @@ -871,5 +871,15 @@ service WorkflowService { | |||
| }; | |||
| } | |||
|
|
|||
| // DescribeActivityById is called by the client to get activity and for pending and completed activities. | |||
There was a problem hiding this comment.
Docstring can be improved here.
Please also mention that an activity ID isn’t unique for an entire workflow run only for pending activities and that the API will return the latest matching activity for the given ID. May also want to consider letting a user use the scheduled event ID in some cases because that is unique for the entire run.
There was a problem hiding this comment.
Add more info. I will not add 'event id' as a routing parameter. It if comes to that users can as well read the history.
There was a problem hiding this comment.
I modify the docstring. At this point I don't want to use scheduled event ID as a routing parameter. For this they can get history
There was a problem hiding this comment.
Works for me, just call out the behavior in the docstring please and make sure you test it when you get to the implementation.
| // More detailed state of an activity. | ||
| enum ActivityState { | ||
| ACTIVITY_STATE_UNSPECIFIED = 0; | ||
| // Activity was scheduled but not yet started. |
There was a problem hiding this comment.
| // Activity was scheduled but not yet started. | |
| // Activity was scheduled but the first attempt has not started. |
The only reason I clarify here is because it could be a bit confusing that pre-first-attempt is scheduled but pre-second-attempt is backing off
There was a problem hiding this comment.
we have a separate flag for "backing off". state transition is "backing off"=>"scheduled"-"started"->"completed" or "backing off"
There was a problem hiding this comment.
I am surprised it starts as backing off, but otherwise makes sense.
| // Activity has timed out. | ||
| ACTIVITY_STATE_TIMED_OUT = 7; | ||
| // Activity has been cancelled. | ||
| ACTIVITY_STATE_CANCELLED = 8; |
There was a problem hiding this comment.
| ACTIVITY_STATE_CANCELLED = 8; | |
| ACTIVITY_STATE_CANCELED = 8; |
We use the one-L form in the history message name.
Add definition for DescribeActivityById APi.
Why?
Part of Activity API work.
Breaking changes
Server PR