Fixing default values in the memcache driver. #249

Closed
wants to merge 5 commits into
from

Projects

None yet

5 participants

@gaker
Contributor
gaker commented Aug 21, 2011

Fixing the default array for addservers()

Contributor
ckdarby commented Jun 8, 2012

@narfbg Looking at the current development branch the get() function looks solved & the $cache_server issue looks solved with the current $_memcache_conf

Agree? Close?

Contributor
narfbg commented Jun 8, 2012

@ckdarby I'm not familiar with Memcached, but the get() method doesn't look anything like the one proposed here.

Contributor

Hey @gaker, not sure how I missed this for so long but now that it's being discussed, could you pull the changes and fix any conflicts?

Contributor
cryode commented Oct 13, 2012

@gaker Hey Greg - can you update this to be compatible with the dev branch?

Contributor
gaker commented Oct 13, 2012

I haven't used CI in well over a year, so it's probably best if someone else does it.

-Greg

Sent from my mobile. Please excuse typos.

On Oct 13, 2012, at 4:14, Eric Roberts notifications@github.com wrote:

@gaker Hey Greg - can you update this to be compatible with the dev branch?


Reply to this email directly or view it on GitHub.

Contributor
cryode commented Oct 15, 2012

I was going to port over Greg's changes to an up-to-date pull request, however I'm not too sure this is bug free.

Both of the following appear to be potential issues (both in _setup_memcached()):

A. A matching config file is found, but it is not in the proper array format as exampled. The array_key_exists() checks that begin roughly around line 208 would throw errors.

B. The _memcache_conf property in the class itself has the default_ prefix on each config item. However, in the supplied config file, there is no prefix. If a config file is loaded, the property is automatically set to NULL before having the config file items written to it. This removes the default prefixed config items that are referenced around line 210.

Both of these are relatively easy fixes, but since I do not have Memcache available locally for testing, I don't want to implement these without testing. If someone else has the ability to fix and verify both Greg's additions and these potential bugs, that'd be super. Otherwise I'll try to get Memcache going in my downtime at some point and get to it.

@narfbg narfbg closed this Jul 18, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment