Let's actually use redis right!

[jowrjowr]

This PR fixes the redis cache such that the setex() cache insertion call is actually invoked (it was not previously) and that it sets an appropriate TTL for both the "max-age" and "expires" headers.

Tests are passing for python 2.7 and 3.5. YMMV for others. Then again the build is currently breaking so I can't make that worse!

[jowrjowr]

There's a weird issue with the setex() command though.

Testing worked fine with setex(key, value, expires) just like the the documentation says https://redis-py.readthedocs.io/en/latest/index.html#redis.Redis.setex

Except it fails with "value is not an integer or out of range" in production. But not testing. So I swap the order back to the 'wrong' value, and it works in production. WTF?

[ionrock]

The if not headers or 'data' no in headers on line 157 should ensure you have a headers['date'] to parse. If you need a default date, then I'd move it before line 157. Does that make sense?

[ionrock]

Let's remove the extra whitespace here so it feels connected to the following code.

[ionrock]

Let's skip the extra whitespace in order to keep the if block connected with the following code.

[ionrock]

It would be worth adding a comment that the file cache doesn't use the expires argument and its inclusion in the API is to support data stores that provide expiration. The file cache is used pretty heavily, so making a note for future readers would be appreciated.

[ionrock]

That error in CI is due to a test cherrypy server not being happy in the travis environment. I haven't had a chance to dig in.

Regarding the redis issue, I'd stick with what the docs say is correct and try to figure out what value you might be getting that is causing the problem. Switching the args is probably a clue!

When you get it sorted, please make sure to organize your commits and remove any 'trying this out' commits with a clear story of what you did. Squashing is OK too :)

Thanks for digging into this!

[jowrjowr]

So this finally bit me hard and now I understand the issue. In short, pyredis does a really fucked up thing.

They have two redis clients: "Redis" and "StrictRedis". Redis uses key, value, expires whereas StrictRedis uses key, expires, value as the data tuple. So when I said "the documentation is inconsistent" I was half right - I was using the first setex() I found because why on earth would it be different?!

I'm now using a code block like this to differentiate:

        if redis_class == 'redis.client.StrictRedis':
            # StrictRedis
            self.conn.setex(key, expires, value)
        elif redis_class == 'redis.client.Redis':
            # Redis
            self.conn.setex(key, value, expires)
        else:
            # unknown redis client type
            self.conn.set(key, value)

It really frustrates me that I have to do this. But not just me: https://github.com/andymccurdy/redis-py/issues?utf8=%E2%9C%93&q=setex

Read that for some "laughs".

I might do an updated PR but I'm maintaining my own build of this on jowrjowr/cachecontrol that has the zlib compression as well.

[alastairmccormack]

Why can't we use kwargs instead of positional args regardless of Redis vs StrictRedis? E.g:

self.conn.setex(key=key, expires=expires, value=value)

[ionrock]

I'm closing this one as I'd much prefer someone else write a better redis cache backend then mess with this toy implementation.