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

Refs #22338 - add worker_timeout param #231

Merged
merged 1 commit into from
Jan 24, 2018
Merged

Refs #22338 - add worker_timeout param #231

merged 1 commit into from
Jan 24, 2018

Conversation

chris1984
Copy link
Member

No description provided.

@@ -75,6 +75,10 @@
#
# $rest_client_timeout:: Timeout for Katello rest API
#
# $pulp_worker_timeout:: The amount of time (in seconds) before considering a worker as missing. If Pulp's
# mongo database has slow I/O, then setting a higher number may resolve issues where workers are
# going missing incorrectly. Defaults to 30.
Copy link
Member

Choose a reason for hiding this comment

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

The default in params.pp is 60. Not sure which we should default to but they should match :)

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

You don't actually pass the parameter to the pulp class either. Will also need theforeman/puppet-pulp#299

@@ -89,6 +93,7 @@
String $qpid_interface = $::katello::params::qpid_interface,
String $qpid_hostname = $::katello::params::qpid_hostname,
Integer[1] $num_pulp_workers = $::katello::params::num_pulp_workers,
Integer[0] $pulp_worker_timeout = $katello::params::pulp_worker_timeout,
Copy link
Member

Choose a reason for hiding this comment

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

Do you mind also properly namespacing this

$::katello::params::pulp_worker_timeout,

@chris1984
Copy link
Member Author

@ekohl can I get another review please?

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

I think you still need to pass this into the pulp class in manifests/pulp.pp

@chris1984
Copy link
Member Author

@ekohl can you review again when you get a moment please, tests are failing for a different reason.

@ekohl
Copy link
Member

ekohl commented Jan 23, 2018

Looks like we need to pin rdoc in modulesync :(

@chris1984 chris1984 merged commit 00822e5 into theforeman:master Jan 24, 2018
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.

4 participants