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

Adds check to params.pp if lab-release is not installed #329

Merged
merged 1 commit into from Jul 29, 2014

Conversation

spuder
Copy link

@spuder spuder commented Jul 10, 2014

Adds spec test

If lsb-release is not installed, then the end user sees a confusing/ vague message

Error: Unsupported lsbdistid () at /modules/apt/manifests/params.pp:52

It is common for docker containers to not include this package by default

After fix, the user sees a friendlier message if lab-release is not installed

Error: Unable to determine lsbdistid, is lsb-release installed? at /modules/apt/manifests/params.pp:52

Adds spec test

If lab-release is not installed, then the end user sees a confusing/ vague message
Error: Unsupported lsbdistid () at /modules/apt/manifests/params.pp:52
It is common for docker containers to not include this package by default

After fix, the user sees a friendlier message if lab-release is not installed
Error: Unable to determine lsbdistid, is lsb-release installed? at /modules/apt/manifests/params.pp:52
@daenney
Copy link

daenney commented Jul 10, 2014

Great, thank you for this. I've been battling to get lsb-release as a dependency for Facter in the Debian packages but no luck so far.

@spuder
Copy link
Author

spuder commented Jul 10, 2014

Instead of using lsbdist, I think it would be better to use $::osfamily and $::operatingsystem or $::operatingsystemrelease since they have been around since facter 1.6

http://serverfault.com/questions/478914/puppet-facts-should-we-use-lsbdistid-or-operatingsystem/479055#479055

http://jenkner.org/blog/2013/03/27/use-osfamily-instead-of-operatingsystem/

Because so many people depend on this module, I didn't want to make such a big rewrite.

@daenney
Copy link

daenney commented Jul 10, 2014

That might not be a bad idea though people usually know Debian releases only by code name (Squeeze) or designation (old-stable), not by version (contrary to Ubuntu because Ubuntu has really crazy unpronounceable code names at times).

I'm gearing up for a serious rewrite of this module in preparation for Puppet 4 so I'll keep that in mind.

underscorgan pushed a commit that referenced this pull request Jul 29, 2014
Adds check to params.pp if lab-release is not installed
@underscorgan underscorgan merged commit 256a63b into puppetlabs:master Jul 29, 2014
@underscorgan
Copy link

👍

Thanks for improving error messages @spuder!

@spuder spuder deleted the spuder/lsbdist branch July 30, 2014 15:32
@LukasAud LukasAud added the bugfix label Jun 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants