saz / puppet-php

Puppet module to manage php (Apache, CLI, FPM)
Other
64 stars 38 forks source link

Provide ability to specify a specific version of PHP to install #12

Open rtgibbons opened 11 years ago

rtgibbons commented 11 years ago

Got the idea from - http://docs.puppetlabs.com/puppet/2.7/reference/lang_classes.html#defining-a-class

Please let me know what you think.

saz commented 11 years ago

Most changes seems to rename $ensure to $version. Is there any specific reason for this, as this change will break the module for all people already using it and defining ensure.

Most of the functionality you're trying to implement is already possible with the ensure parameter. I'd rather like to see this functionality fixed/improved.

rtgibbons commented 11 years ago

First off, I need to clean up this pull request. I started putting too much work into this one branch and forgot I had the pull request set.

Secondly, there wasn't an $ensure variable being used. The class to install the package was hardcoded to ensure => present. I wanted to be able to load the php-fpm class and pass in a version number to a specific version. I've set the default to be present, so it won't break for everyone.

So now, for install php-fpm I can call

class { 'php-fpm':
  version => '5.4.8-precise~1'
}

If there was a way to override the below code in another way I'd like to hear how.

package { 'php-fpm':
  ensure => present
} 
rtgibbons commented 11 years ago

Oh, I see where I change the $ensure for the php::module. I do this based on the documentation from puppetlabs, and just keeping with their practices. We could probably leave the $ensure and still have the $version, and use $version if it's not undef, and otheruse use $ensure

rtgibbons commented 11 years ago

According to the documentation in the link, you can set the ensure to the version of the package you want and it will install that packaged version

saz commented 11 years ago

What about using ensure rather then version and implementing the missing parts? I think this is the best solution, as this won't change anything for old installations and you're able to specify a specific version.

What do you mean?

rtgibbons commented 11 years ago

Yeah, that would work. Again I was just basing it off the precedence set by the docs at http://docs.puppetlabs.com/puppet/2.7/reference/lang_classes.html#defining-a-class

rtgibbons commented 11 years ago

I'll get this code cleaned up sticking with the standards you've already set

saz commented 11 years ago

Great. Thanks!

Stoosh commented 11 years ago

Was just looking around for a solution to this. Was this resolved/is this achievable with the master?

Happy to help where I can.

rtgibbons commented 11 years ago

No, I got distracted with the rest of our roll our and forget. I need to clean up the variable usage for ensure.

CpuID commented 10 years ago

Any progress on this one? hard locking versions is a big positive once its in...

rtgibbons commented 10 years ago

No, our department decided not to go with Puppet and I haven't had time to go back. It still needs the variable clean up as suggested by saz