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

Add support for specifying unsupported repo location. #612

Merged
merged 3 commits into from
Jun 10, 2019

Conversation

jadestorm
Copy link

Pull Request (PR) description

This fairly simply PR adds the ability to specify the "unsupported" repo path similar to how you can override the repo_path. (zabbix::repo::unsupported_repo_path is the new setting) I am using this a a temporary measure to get Zabbix running on some RHEL 8 agents before RHEL 8 is even available, but I imagine it could be useful to others who want to mirror the unsupported repo as well. At some level I figured, you can configure one, why not the other. =)

This Pull Request (PR) fixes the following issues

n/a

@jadestorm
Copy link
Author

Er, zabbix::repo::unsupported_repo_location not path =)

manifests/params.pp Outdated Show resolved Hide resolved
@bastelfreak bastelfreak added the enhancement New feature or request label Jun 8, 2019
@bastelfreak bastelfreak changed the title Add support for specifying unsupported repo path. Add support for specifying unsupported repo location. Jun 8, 2019
@bastelfreak bastelfreak added the needs-work not ready to merge just yet label Jun 8, 2019
manifests/repo.pp Outdated Show resolved Hide resolved
manifests/repo.pp Outdated Show resolved Hide resolved
@jadestorm
Copy link
Author

@bastelfreak I pushed up an update that includes the fixes you requested. Tests don't pass but they look like they may have been busted for quite some time. Could you please confirm that I didn't cause these?

Otherwise, see anything else that needs a tweak? =) Thanks!

@bastelfreak bastelfreak removed the needs-work not ready to merge just yet label Jun 10, 2019
String[1] $zabbix_version = $zabbix::params::zabbix_version,
Boolean $manage_repo = $zabbix::params::manage_repo,
Boolean $manage_apt = $zabbix::params::manage_apt,
Stdlib::HTTPUrl $repo_location = $zabbix::params::repo_location,
Copy link
Member

Choose a reason for hiding this comment

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

those types need to be optional

Suggested change
Stdlib::HTTPUrl $repo_location = $zabbix::params::repo_location,
Optional[Stdlib::HTTPUrl] $repo_location = $zabbix::params::repo_location,

manifests/repo.pp Outdated Show resolved Hide resolved
@bastelfreak
Copy link
Member

The tests are failing because the new code always requires a value for both variables. They need to be optional (sorry, I missed that during my last review).

@jadestorm
Copy link
Author

oh! my bad just a moment.

@jadestorm
Copy link
Author

I went ahead and make repo_location optional as well since it "really is" .
Keeping an eye on the tests.

@jadestorm
Copy link
Author

Much better =)

@bastelfreak
Copy link
Member

Thanks for the PR!

@bastelfreak bastelfreak merged commit 8b314c9 into voxpupuli:master Jun 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants