-
Notifications
You must be signed in to change notification settings - Fork 139
Let's actually use redis right! #168
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
Changes from all commits
75550b9
867772b
8d4d2c3
2ff26d9
0c0683d
edc5bb8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,5 @@ | ||
| .cache | ||
| web_cache | ||
| .DS_Store | ||
| *.pyc | ||
| *.pyo | ||
|
|
@@ -10,4 +12,4 @@ include | |
| .Python | ||
| docs/_build | ||
| build/ | ||
| .tox | ||
| .tox | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,9 +26,24 @@ def set(self, key, value, expires=None): | |
| if not expires: | ||
| self.conn.set(key, value) | ||
| else: | ||
| expires = expires - datetime.utcnow() | ||
| self.conn.setex(key, total_seconds(expires), value) | ||
|
|
||
| # the keyword arguments are to account for a Redis v StrictRedis issue | ||
| # with pyredis being a mess. this is compatible with both. | ||
|
|
||
| redis_class = self.conn.__class__ | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why can't we use kwargs instead of positional args regardless of Redis vs StrictRedis? E.g:
|
||
| 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. give it a shot. | ||
|
|
||
| try: | ||
| self.conn.setex(key, expires, value) | ||
| except Exception as e: | ||
| # complete failure. give up and don't set a date. | ||
| self.conn.set(key, value) | ||
| def delete(self, key): | ||
| self.conn.delete(key) | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -164,9 +164,14 @@ def cached_request(self, request): | |
| return False | ||
|
|
||
| now = time.time() | ||
| date = calendar.timegm( | ||
| parsedate_tz(headers['date']) | ||
| ) | ||
|
|
||
| if 'date' in headers: | ||
| date = calendar.timegm( | ||
| parsedate_tz(headers['date']) | ||
| ) | ||
| else: | ||
| date = 0 | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
| current_age = max(0, now - date) | ||
| logger.debug('Current age based on date: %i', current_age) | ||
|
|
||
|
|
@@ -242,6 +247,7 @@ def conditional_headers(self, request): | |
|
|
||
| def cache_response(self, request, response, body=None, | ||
| status_codes=None): | ||
|
|
||
| """ | ||
| Algorithm for caching requests. | ||
|
|
||
|
|
@@ -260,6 +266,16 @@ def cache_response(self, request, response, body=None, | |
|
|
||
| response_headers = CaseInsensitiveDict(response.headers) | ||
|
|
||
| # what time is it now and from the header | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's remove the extra whitespace here so it feels connected to the following code. |
||
| now = time.time() | ||
| if 'date' in response_headers: | ||
| date = calendar.timegm( | ||
| parsedate_tz(response_headers['date']) | ||
| ) | ||
| else: | ||
| date = 0 | ||
|
|
||
| # If we've been given a body, our response has a Content-Length, that | ||
| # Content-Length is valid then we can check to see if the body we've | ||
| # been given matches the expected size, and if it doesn't we'll just | ||
|
|
@@ -291,9 +307,22 @@ def cache_response(self, request, response, body=None, | |
| # If we've been given an etag, then keep the response | ||
| if self.cache_etags and 'etag' in response_headers: | ||
| logger.debug('Caching due to etag') | ||
|
|
||
| if response_headers.get('expires'): | ||
| expires = parsedate_tz(response_headers['expires']) | ||
| if expires is not None: | ||
| expire_time = calendar.timegm(expires) - date | ||
| else: | ||
| expire_time = 0 | ||
|
|
||
| expire_time = max(expire_time, 14*86400) | ||
|
|
||
| logger.debug('etag object cached for {0} seconds'.format(expire_time)) | ||
|
|
||
| self.cache.set( | ||
| cache_url, | ||
| self.serializer.dumps(request, response, body=body), | ||
| expires=expire_time | ||
| ) | ||
|
|
||
| # Add to the cache any 301s. We do this before looking that | ||
|
|
@@ -315,16 +344,25 @@ def cache_response(self, request, response, body=None, | |
| self.cache.set( | ||
| cache_url, | ||
| self.serializer.dumps(request, response, body=body), | ||
| expires=cc['max-age'], | ||
| ) | ||
|
|
||
| # If the request can expire, it means we should cache it | ||
| # in the meantime. | ||
| elif 'expires' in response_headers: | ||
| if response_headers['expires']: | ||
| logger.debug('Caching b/c of expires header') | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's skip the extra whitespace in order to keep the if block connected with the following code. |
||
| expires = parsedate_tz(response_headers['expires']) | ||
| if expires is not None: | ||
| expire_time = calendar.timegm(expires) - date | ||
| else: | ||
| expire_time = None | ||
|
|
||
| logger.debug('Caching b/c of expires header. expires in {0} seconds'.format(expire_time)) | ||
| self.cache.set( | ||
| cache_url, | ||
| self.serializer.dumps(request, response, body=body), | ||
| expires=expire_time, | ||
| ) | ||
|
|
||
| def update_cached_response(self, request, response): | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.