Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set default OrchestrationTemplateRunner timeout to 100 minutes. #19381

Merged
merged 1 commit into from
Oct 14, 2019

Conversation

lfu
Copy link
Member

@lfu lfu commented Oct 9, 2019

To match the default automate timeout.

https://bugzilla.redhat.com/show_bug.cgi?id=1756292

@miq-bot assign @tinaafitz
@miq-bot add_label bug, services

@miq-bot
Copy link
Member

miq-bot commented Oct 9, 2019

@lfu Cannot apply the following label because they are not recognized: service

@miq-bot miq-bot added the bug label Oct 9, 2019
@@ -1,5 +1,5 @@
class ManageIQ::Providers::CloudManager::OrchestrationTemplateRunner < ::Job
DEFAULT_EXECUTION_TTL = 10 # minutes
DEFAULT_EXECUTION_TTL = 100 # minutes
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you change this constant name to include minutes, perhaps DEFAULT_EXECUTION_TTL_MINUTES? We generally try to store the base unit of measure: seconds, bytes, etc. I would not expect this to be in minutes.

@lfu lfu force-pushed the orch_stack_timeout_1756292 branch from 6317702 to c5e2dc1 Compare October 10, 2019 21:19
@miq-bot
Copy link
Member

miq-bot commented Oct 10, 2019

Checked commit lfu@c5e2dc1 with ruby 2.4.6, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 1 offense detected

app/models/manageiq/providers/cloud_manager/orchestration_template_runner.rb


def minimize_indirect
@minimize_indirect = true if @minimize_indirect.nil?
@minimize_indirect
end

def current_job_timeout(_timeout_adjustment = 1)
@execution_ttl ||=
(options[:execution_ttl].present? ? options[:execution_ttl].try(:to_i) : DEFAULT_EXECUTION_TTL) * 60
@execution_ttl ||= options[:execution_ttl].present? ? options[:execution_ttl].to_i.minutes : DEFAULT_EXECUTION_TTL
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes a lot more sense to me, thanks @lfu

Copy link
Member

@jrafanie jrafanie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jrafanie jrafanie merged commit b61deda into ManageIQ:master Oct 14, 2019
@jrafanie jrafanie added this to the Sprint 122 Ending Oct 14, 2019 milestone Oct 14, 2019
@lfu lfu deleted the orch_stack_timeout_1756292 branch November 4, 2019 15:22
@simaishi
Copy link
Contributor

simaishi commented Jan 6, 2021

@lfu Can this be ivanchuk/yes?

@lfu
Copy link
Member Author

lfu commented Jan 7, 2021

@simaishi Yes.

simaishi pushed a commit that referenced this pull request Jan 7, 2021
Set default OrchestrationTemplateRunner timeout to 100 minutes.

(cherry picked from commit b61deda)

https://bugzilla.redhat.com/show_bug.cgi?id=1756292
@simaishi
Copy link
Contributor

simaishi commented Jan 7, 2021

Ivanchuk backport details:

$ git log -1
commit 97a14bbcb340c5d918940e523aef6cdf38557f7d
Author: Joe Rafaniello <[email protected]>
Date:   Mon Oct 14 10:51:14 2019 -0400

    Merge pull request #19381 from lfu/orch_stack_timeout_1756292

    Set default OrchestrationTemplateRunner timeout to 100 minutes.

    (cherry picked from commit b61deda502d3285f318c8b51259ab18f1560fe25)

    https://bugzilla.redhat.com/show_bug.cgi?id=1756292

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants