adding simple polling timeout mechanism #151
Conversation
|
example of a more advanced implementation: Katello/katello#5225 this uses process_timeout to do a 2-phase timeout |
| @@ -13,6 +14,9 @@ def run(event = nil) | |||
| end | |||
| when Poll | |||
| poll_external_task_with_rescue | |||
| when Timeout | |||
| process_timeout | |||
| poll_external_task_with_rescue | |||
There was a problem hiding this comment.
This line is unreachable, rigth (since the process_timeout will fail)
There was a problem hiding this comment.
Only if process timeout throws an exception (in the default case that is true, but in the 'advanced' cases where process_timeout is overridden, maybe not).
There was a problem hiding this comment.
shouldn't that then be part of the overridden process_timeout the responsibility to poll?
|
Looks good. One suggestion: extracting the timeout-specific methods into |
|
Would you mind adding some tests into https://github.com/Dynflow/dynflow/blob/master/test/action_test.rb, you can inspire by https://github.com/Dynflow/dynflow/blob/master/test/action_test.rb#L256: simply defining a dummy polling action that would define the timeouts and would simulate the hanging, and then checking that it got paused after the timeout (the timeout might be really short so that the tests are still fast) |
|
The tests would then be also a good example on how to use this feature |
jlsherrill
force-pushed
the
content_timeout
branch
from
1298630
to
bcbeb83
Compare
This adds the ability for an action to call schedule_timeout(50), to schedule when the polling should stop and the task be marked as failed. In this simple implementation the above would cause the task to fail after 50 seconds. Actions could override process_timeout to do more advanced things
Before this patch, the clock in tests were not respecting the time values, so some event to happen in far future would be triggered before the shot-term event. Also, when two events were pending in clock, when while processing the first event, some other event occurred, we were ignoring it. Now, the new event gets properly scheduled.
jlsherrill
force-pushed
the
content_timeout
branch
from
bcbeb83
to
5829fea
Compare
|
Thanks @jlsherrill |
adding simple polling timeout mechanism
This adds the ability for an action to call schedule_timeout(50), to
schedule when the polling should stop and the task be marked as failed.
In this simple implementation the above would cause the task to fail after 50
seconds. Actions could override process_timeout to do more advanced things