Conversation
Signed-off-by: Tihomir Surdilovic <tihomir@temporal.io>
antmendoza
left a comment
antmendoza
left a comment
There was a problem hiding this comment.
thanks @tsurdilo, disregard my previous comment (deleted). I got confused with the other PR that uses the same branch name
| } | ||
| } | ||
| } | ||
| return super.execute(input); |
There was a problem hiding this comment.
I think the activity function is executed twice when there is a heartbeat, here and in line 74 of this same file
| return super.execute(input); | ||
| } | ||
|
|
||
| public interface AutoHeartbeaterCancellationCallback { |
There was a problem hiding this comment.
this is not used, I believe it can be deleted
|
thanks for review @antmendoza . will update over weekend |
|
applied updates |
| @@ -0,0 +1,16 @@ | |||
| # Auto-heartbeating sample for activities that define HeartbeatTimeout | |||
There was a problem hiding this comment.
This sample should be linked from the top level readmen
| if (activityHeartbeatTimeout != null | ||
| && activityHeartbeatTimeout.getSeconds() > 0 | ||
| && !Boolean.parseBoolean(System.getProperty("sample.disableAutoHeartbeat"))) { | ||
| System.out.println( |
There was a problem hiding this comment.
should be a logger no?
There was a problem hiding this comment.
That applies to pretty much all logging in this sample
There was a problem hiding this comment.
@Quinn-With-Two-Ns system outs are either in activity or activity interceptor. if you want we can add activity logger
There was a problem hiding this comment.
They should still use an sl4j logger
| } | ||
|
|
||
| @Override | ||
| public void cancelActivity() { |
There was a problem hiding this comment.
Why cancel through a signal and not just use normal workflow cancellation/.
There was a problem hiding this comment.
yeah will add a "run" for workflow cancelation too. forgot i think
|
@tsurdilo we tested this and I think the activity continues running after the cancellation request is received and the heartbeat stops. I am not sure if there is a way to stop it, @Quinn-With-Two-Ns might have some input |
|
@antmendoza The users activity code has to choose to stop, you would need to add a mechanism for the auto heartbeat to "signal" the activity to stop and the activity has to respond to that. This is why we don't really recommend auto heartbeating, it isn's some magic solution, users code still needs to handle cancellation. |
Signed-off-by: Tihomir Surdilovic <tihomir@temporal.io>
|
added a "warning" message in sample readme to make sure users are aware of the pitfalls of autoheartbeating approach |
Signed-off-by: Tihomir Surdilovic <tihomir@temporal.io>
3 participants
autoheartbeat sample ..support cancelation