Checking for memcached extension

[garethtdavies]

Hi,

Firstly thanks for this package, it works great in production for me,

I have an issue where on my local environment which is Windows I get a 'Class Memcached Not Found'. This breaks the entire application as the error occurs in the service provider. It's not possible to install the Memcached.dll extension on Windows but as I am not using the memcached driver in this environment (it's set to file) I don't want my application to fall over due to a missing driver that I'm not using.

I've patched this bug with the following simple check to test for the extension. There might be a better way of doing it but this works for me!

[coveralls]

Coverage remained the same at 100.0% when pulling 355a788f059685e4eb92f0703b54f2aae98099bd on garethtdavies:master into cf86fad65318ae991a6337b2bad89eb52f077eb1 on atyagi:master.

[atyagi]

Hi @garethtdavies ,

Thanks for the PR! Sounds like the problem is more that the Service Provider is being loaded when it shouldn't be since you're using a file driver locally.

Personally, I think the service provider should fail if the memcached extension is not available, but the service provider shouldn't load if there's no need to connect to memcached.

I have a feeling it might be tied to https://github.com/atyagi/elasticache-laravel/blob/master/src/Atyagi/Elasticache/ElasticacheServiceProvider.php#L12 - Do you think you could change that variable to true and see if it works for your specific case? I'd prefer to change that instead of adding the extension check as this would also eliminate unnecessary connections to memcached overall.

Thank you!

[garethtdavies]

Hi @atyagi,

Thanks for the reply and completely agree with you - in an ideal world it would never load the service provider.

I've tried deffering and no luck. It does have an effect on the storage/meta/services.json file but doesn't stop the service provider from loading. I'll do a bit more research and see what I can come up.

[atyagi]

Hey @garethtdavies ,

Thanks for testing quickly. I definitely want to investigate more as well and see if we can find a better solution.

Thanks!

[garethtdavies]

@atyagi I think the key line in http://laravel.com/docs/master/providers#deferred-providers is 'If your provider is only registering bindings' - note the only. So it's not being deferred.

I can also get around it by doing a check on the driver of the session and cache i.e. if($this->app['config']->get('session.driver')=="memcached") which at least would stop it being loaded if the extension was available but not being used if there is not a better solution.

[atyagi]

Hey @garethtdavies , I agree with that - I think because of the session extension it needs to boot by default on every request.

The best approach would probably be to go with your approach on the ElasticacheConnecter, but in the Service Provider run a check based on what the connecter returns to do the actual bindings.

Thanks for all the help! Much appreciated!

[atyagi]

Hey @garethtdavies ,

I went ahead and added your check for version 1.1.3 - please see https://github.com/atyagi/elasticache-laravel/commit/2f7edb3206dbe9d705ed2ccfd35acdfa0084711c for the commit info.

Thanks again for bringing this up!

[garethtdavies]

Great thanks. Having trouble testing it as it requires ext-memcached so I can't install it on the windows box!

atyagi/elasticache-laravel 1.1.3 requires ext-memcached * -> the requested PHP extension memcached is missing from your system.

[garethtdavies]

Hey @atyagi,

Thanks again. I have this working by forking your repo and then pulling directly from this fork in my composer file. I had to make 2 changes to get this to work which you can see here https://github.com/garethtdavies/elasticache-laravel basically removing the requirement of loading ext-memcached and then as a result returning return new \Memcached; in the global namespace as this no longer worked and I got a class not found exception otherwise.

This of course then means that you need to have the memcached extension separately installed.

[atyagi]

Hey @garethtdavies ,

Thanks for the suggestions! I actually updated the composer.json file to remove ext-memcached requirement and simply have it be suggested. I also added the global namespace as part of 1.1.5 so you can reference that as well.

Thanks again for all the input and insight! Truly appreciate it.

[garethtdavies]

@atyagi Great stuff - switched to using this and there is a bug on line 26 of ElasticacheServiceProvider.php - it should be if(memcached) rather than if(!memcached) as you clearly want that block to run only if we have the extension.

[atyagi]

@garethtdavies Thanks again for the catch - fixed as part of version 1.1.6

[garethtdavies]

@atyagi perfect. Tested on Windows and Linux box (with the memcached extension) and it's working!

Thanks for all your help and the code. Please have a proverbial beer on me @ChangeTip

[changetip]

Hi @atyagi, @garethtdavies sent you a Bitcoin tip worth 1 beer (15,009 bits/$3.50), and I'm here to deliver it ➔ collect your tip.

Learn more about ChangeTip

[atyagi]

Hahaha, love it. Thanks @garethtdavies !