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

mount should not default to an empty array. #5

Closed
alexjfisher opened this issue Jan 6, 2017 · 2 comments
Closed

mount should not default to an empty array. #5

alexjfisher opened this issue Jan 6, 2017 · 2 comments
Assignees
Labels
bug Something isn't working

Comments

@alexjfisher
Copy link
Member

alexjfisher commented Jan 6, 2017

$mount = hiera_hash('autofs::mounts', [])
looks funky.

$mount = hiera_hash('autofs::mounts', [])

$mount is passed to create_resources which takes a hash as its second argument, not an array.
I think the correct change isn't

$mount = hiera_hash('autofs::mounts', {})

but perhaps

$mount = hiera_hash('autofs::mounts', $::autofs::mounts)

In this way, the module would work for users who have passed a mounts hash to the base class explicitly as well as those using hiera.

There's also every chance I've just misread the code and this isn't a bug/issue at all! :)

@alexjfisher
Copy link
Member Author

https://www.devco.net/archives/2016/03/13/the-puppet-4-lookup-function.php might also be worth a look at. I've not yet personally used it, but I think it's now possible to do automatic parameter lookups of hash parameters from hiera with merging.

@dhollinger
Copy link
Member

The code was originally designed only with hieradata in mind with the assumption that if a user wasn't using hieradata, they'd directly call the defined type.

In fact, I "stole" the idea from Gareth's Docker module's images.pp.

The use of hiera_hash was proposed in a PR back in October to account for repeating keys and native merging. After reviewing it, I merged it. That said, my understanding of hiera_hash is limited as I haven't had to use it yet, nor do I have a lot of experience with the lookup function yet

@dhollinger dhollinger added the enhancement New feature or request label Jan 14, 2017
@dhollinger dhollinger self-assigned this Jan 14, 2017
@dhollinger dhollinger added bug Something isn't working and removed enhancement New feature or request labels Jan 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants