paquettg / leaguewrap

League of Legend API wrapper
MIT License
68 stars 28 forks source link

Fixed memcached expiration time #91

Closed m1so closed 8 years ago

m1so commented 8 years ago

There seems to be an error in Memcached documentation regarding expiration times (see this comment)

If you want to test it yourself have a look at: http://cl.ly/dYd8

dnlbauer commented 8 years ago

From my understanding of the documentation, both should work. If the passed time in seconds is smaller than 30 days, memcached will consider it as a relative time, while it uses it as a unix time when its bigger than that:

See: http://php.net/manual/en/memcached.expiration.php

Both versions should give the correct result because of that, although not using time() might be a bit faster.

m1so commented 8 years ago

You can always test it yourself or have a look at the screenshot

<?php
// Memcached_test.php
$m = new Memcached;
$m->addServer('localhost', 11211, 100);
$m->set('key1', 'without time()', 10);
echo time();
$m->set('key2', 'with time()', time()+10);
echo time();
sleep(11);
var_dump($m->get('key2')); // should return "with time()"
var_dump($m->get('key1')); // should return false

Tested on Laravel Homestead - Ubuntu 14.04 running PHP 5.6.14 and the version without time() works perfectly, although it doesn't expire with time() on the date you'd expect.

dnlbauer commented 8 years ago

Already merged it :)

On Mon, Oct 19, 2015, 11:29 AM Michal Baumgartner notifications@github.com wrote:

You can always test it yourself or have a look at the screenshot

<?php// Memcached_test.php$m = new Memcached;$m->addServer('localhost', 11211, 100);$m->set('key1', 'without time()', 10);echo time();$m->set('key2', 'with time()', time()+10);echo time();sleep(11);var_dump($m->get('key2')); // should return "with time()"var_dump($m->get('key1')); // should return false

Tested on Laravel Homestead - Ubuntu 14.04 running PHP 5.6.14 and the version without time() works perfectly, although it doesn't expire with time() on the date you'd expect.

— Reply to this email directly or view it on GitHub https://github.com/paquettg/leaguewrap/pull/91#issuecomment-149158779.