Support TTL (Time To Live) value for services. #148
Conversation
|
@tinaafitz Please Review |
| @@ -17,6 +17,7 @@ def main | |||
| @handle.log('info', msg) | |||
| @handle.create_notification(:level => 'info', :subject => service, :message => msg) | |||
| @handle.root['ae_result'] = 'ok' | |||
| check_interval | |||
There was a problem hiding this comment.
I would not call this method check_interval since it is setting, not checking and the name does not clarify what type of interval. A better name would be calculate_retry_internal and reduce the method to just returning the integer value.
This line would become:
@handle.root['ae_retry_interval'] = calculate_retry_internal
| @@ -30,6 +31,17 @@ def service | |||
| end | |||
| end | |||
|
|
|||
| def check_interval | |||
| return unless @handle.root['ae_retry_ttl'] | |||
There was a problem hiding this comment.
@tinaafitz @billfitzgerald0120 This logic is not taking the current generic state-machine action into account. Is this value being populated during an automate instance resolution?
If the value is coming straight from the service_template (or service) config settings I would not expect it to have the ae_ prefix.
| def check_interval | ||
| return unless @handle.root['ae_retry_ttl'] | ||
|
|
||
| interval = @handle.root['ae_retry_ttl'] / @handle.root['ae_state_max_retries'] |
There was a problem hiding this comment.
We should ensure that both of these values are integers. (.to_i) The ae_state_max_retries value is an optional value for a state machine step, what value would we end up with here when it is blank? Divide by zero? 💥
billfitzgerald0120
force-pushed
the
timetolive
branch
from
bf05465
to
6aeec4b
Compare
|
@gmcculloug Made changes as requested, please review |
billfitzgerald0120
force-pushed
the
timetolive
branch
from
6aeec4b
to
ec2edeb
Compare
|
@tinaafitz |
|
|
||
| def retry_interval | ||
| ttl = playbook_ttl | ||
| return if ttl.zero? || @handle.root['ae_state_max_retries'].zero? |
There was a problem hiding this comment.
Moving @handle.root['ae_state_max_retries'] into a variable will make this method easier to read.
def retry_interval
ttl = playbook_ttl
max_retry_count = @handle.root['ae_state_max_retries']
return if ttl.zero? || max_retry_count.zero?
...
billfitzgerald0120
force-pushed
the
timetolive
branch
from
ec2edeb
to
52e8ce1
Compare
Allow specifying the length of time to allow the check_completed step to run in the generic service state-machines. This will allow long running processes to finish. UI changes are needed for this. https://www.pivotaltracker.com/n/projects/1937537/stories/147361947
billfitzgerald0120
force-pushed
the
timetolive
branch
from
52e8ce1
to
6199d78
Compare
|
Checked commit billfitzgerald0120@6199d78 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 |
Allow specifying the length of time to allow the check_completed step to run in the generic service state-machine. This will allow long running processes to finish. UI changes done. https://www.pivotaltracker.com/n/projects/1937537/stories/147361947 https://bugzilla.redhat.com/show_bug.cgi?id=1459735 Initial PR: ManageIQ#148 Moved interval calculations from start to check_completed. @miq-bot add_label enhancement, services @miq-bot assign @gmcculloug
Allow specifying the length of time to allow the check_completed step to run in the generic service state-machine. This will allow long running processes to finish. UI changes done. https://www.pivotaltracker.com/n/projects/1937537/stories/147361947 https://bugzilla.redhat.com/show_bug.cgi?id=1459735 Initial PR: ManageIQ#148 Moved interval calculations from start to check_completed. @miq-bot add_label enhancement, services @miq-bot assign @gmcculloug
Allow specifying the length of time to allow the check_completed step to run in the generic service state-machine. This will allow long running processes to finish. UI changes done. https://www.pivotaltracker.com/n/projects/1937537/stories/147361947 https://bugzilla.redhat.com/show_bug.cgi?id=1459735 Initial PR: ManageIQ#148 Moved interval calculations from start to check_completed.
Support TTL (Time To Live) value for services. (cherry picked from commit 148e5a6) https://bugzilla.redhat.com/show_bug.cgi?id=1479407
|
Fine backport details: |
Allow specifying the length of time to allow the check_completed step to run in the generic service state-machines.
This will allow long running processes to finish.
UI changes are needed for this.
https://www.pivotaltracker.com/n/projects/1937537/stories/147361947
https://bugzilla.redhat.com/show_bug.cgi?id=1459735
@miq-bot add_label enhancement, services
@miq-bot assign @gmcculloug