Skip to content

add compile_mode parameter to puppetserver.conf#574

Merged
mmoll merged 1 commit intotheforeman:masterfrom
miksercz:master
Jan 11, 2018
Merged

add compile_mode parameter to puppetserver.conf#574
mmoll merged 1 commit intotheforeman:masterfrom
miksercz:master

Conversation

@miksercz
Copy link
Copy Markdown
Contributor

@miksercz miksercz commented Jan 9, 2018

Hello,
recently I wanted to try setting puppetserver's compile-mode to some value other than default, your module doesn't as of yet have this functionality. This PR should add it.

@miksercz
Copy link
Copy Markdown
Contributor Author

miksercz commented Jan 9, 2018

I am not sure how could my simple change fail so many tests - will take a closer look at it, when I have the time.

Copy link
Copy Markdown
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.

We test the class in spec/classes/puppet_server_puppetserver_spec.rb and set all the parameters explicitly. Please add the variable to default_params there as well.

Other than that I think it looks good.

Optional[Array] $server_metrics_allowed = $::puppet::params::server_metrics_allowed,
Boolean $server_puppetserver_experimental = $puppet::params::server_puppetserver_experimental,
Array[String] $server_puppetserver_trusted_agents = $puppet::params::server_puppetserver_trusted_agents,
Variant[Undef, Enum['off'], Enum['jit'], Enum['force']] $server_compile_mode = $puppet::params::server_compile_mode,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This type can be Optional[Enum['off', 'jit', 'force']]

Variant[Undef, Array] $metrics_allowed = $::puppet::server_metrics_allowed,
Boolean $puppetserver_experimental = $::puppet::server_puppetserver_experimental,
Array[String] $puppetserver_trusted_agents = $::puppet::server_puppetserver_trusted_agents,
Variant[Undef, Enum['off'], Enum['jit'], Enum['force']] $compile_mode = $::puppet::server_compile_mode,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

See the init.pp type comment.

@ekohl
Copy link
Copy Markdown
Member

ekohl commented Jan 11, 2018

Looks correct, but I'll let Travis finish before approving.

@miksercz
Copy link
Copy Markdown
Contributor Author

@ekohl I believe I named the variable incorrectly in the spec test, it should be called compile_mode, not server_compile_mode. Should be fixed, let's wait for new travis and then I can squash the commits.

@miksercz
Copy link
Copy Markdown
Contributor Author

Problems fixed, commits squashed, please let me know if there's any more changes I should do.

Copy link
Copy Markdown
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.

Looks good. @mmoll?

@mmoll mmoll merged commit 08f9ad9 into theforeman:master Jan 11, 2018
@mmoll
Copy link
Copy Markdown
Contributor

mmoll commented Jan 11, 2018

looks good, merged, díky @miksercz!

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