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 freebsd #79

Merged
merged 1 commit into from
Nov 10, 2016
Merged

add support for freebsd #79

merged 1 commit into from
Nov 10, 2016

Conversation

sethlyons
Copy link
Contributor

No description provided.

@sethlyons
Copy link
Contributor Author

sethlyons commented Sep 28, 2016

fyi, the failed travis jobs aren't from the puppet code; they're from a failure to install certain gems.

@sethlyons
Copy link
Contributor Author

hi -- just wanted to follow up on this.
thanks!

@saz
Copy link
Owner

saz commented Nov 9, 2016

Can you please rebase your PR? I'll merge it asap then.

@olevole
Copy link

olevole commented Nov 9, 2016

@sethlyons Hmm, why this params is hard coded?

'value' => '"-m" "950" "-l" "0.0.0.0" "-p" "11211" "-U" "11211" "-u" "nobody" "-c" "8192" "-t" "1"',

I've try to add FreeBSD support one year ago (see #73 ), but recently I was forced to fork ( https://forge.puppet.com/olevole/memcached/readme ), as I tired to waiting and my clients use FreeBSD a wide

@saz
Copy link
Owner

saz commented Nov 10, 2016

I'm sorry that I'm also limited with my free time. I'm trying to get things in as fast as possible, but sometimes, my priorities are on different things...

Anyway, I'd like to merge this PR and maybe it's useful for you to get rid of your fork.

And to answer your question: those params are hard-coded, because those are tests for the module.
(which is also something, your pull request was lacking, which makes my life harder)

@saz saz merged commit 5467c79 into saz:master Nov 10, 2016
@sethlyons
Copy link
Contributor Author

thanks for merging! possible to do a new release for forge?

@saz
Copy link
Owner

saz commented Nov 10, 2016

@sethlyons Can you give the current master a try if it's working as expected for you?

@olevole
Copy link

olevole commented Nov 10, 2016

@saz I certainly understand that the root of all evil - lake of spare time, we all suffer from this ;-)
But when during the year merged any other pull request except FreeBSD - it makes me think that the for author is inappropriately. Although, we also spend their free time for adaptation, testing and creating a patch to help you make your product better ;-) There is no certainty that any problem on FreeBSD will be corrected in the upstream as fast. Thus FreeBSD patches do not break anything (I hope;)

@sethlyons Unfortunately, your patch is not suitable for my use. How do you change the settings in FreeBSD if you do not create a configuration file to run? For example, with such a configuration as:

        class { 'memcached':
                max_memory => 2048,
                tcp_port => 11211,
                udp_port => 11211,
                max_connections => 512,
                unix_socket => "/tmp/memcached.sock",    <<  i've add unix socket too in my patch
                processorcount => 2,
                use_sasl => true,
        }

memcached launched via puppet in this form:

 /usr/local/bin/memcached -d -u nobody -P /var/run/memcached/memcached.pid

My version of pull/patch create configuration in /etc/rc.conf.d/memcached:

# Added by Puppet
memcached_enable="YES"
memcached_flags="-d -u nobody -P /var/run/memcached/memcached.pid -l 0.0.0.0 -p 11211 -U 11211 -t 2 -c 512 >> /var/log/memcached.log 2>&1 -s /tmp/memcached.sock"

So i sure that memcached started in next time without puppet with corrected settings:

:00.01 /usr/local/bin/memcached -d -u nobody -P /var/run/memcached/memcached.pid -l 0.0.0.0 -p 11211 -U 11211 -t 2 -c 512 -s /tmp/memcached.sock -d -u nobody -P /var/run/memcached/memcached.pid

see my .erb:

https://github.com/olevole/puppet-memcached/blob/master/templates/memcached_freebsd_rcconf.erb

@saz
Copy link
Owner

saz commented Nov 10, 2016

@olevole The missing config file is the reason, I've asked @sethlyons to give the current master a try. But I think, I'll grab this part from your PR to get this working as expected.

And for anything else: Let's stop wasting our time and invest it, to improve this module for FreeBSD ;-)

@sethlyons
Copy link
Contributor Author

i made these changes. they're just about ready but still need some finishing touches. i'll open a new PR later tonight or tomorrow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants