Skip to content
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

Cope with cache connection errors #88

Merged
merged 1 commit into from
Oct 14, 2015

Conversation

tdhooper
Copy link
Contributor

If a memcached were to become unavailable, pages using the decorator would start throwing errors. This change copes with these errors and keeps pages available.

@@ -389,6 +399,37 @@ def do_increment(request):
assert do_increment(req), 'Request should be rate limited.'
assert not_increment(req), 'Request should be rate limited.'

@override_settings(RATELIMIT_USE_CACHE='connection-errors')
def test_is_ratelimited_cache_connection_error(self):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mind dropping most of this test case and just calling is_ratelimited directly? The test it's copied from isn't a great test, and we can cover this case with something much simpler, more like the other test you added.

@jsocol
Copy link
Owner

jsocol commented Oct 6, 2015

Thanks @tdhooper! Just a few issues.

@tdhooper
Copy link
Contributor Author

tdhooper commented Oct 6, 2015

Thanks for the review @jsocol, back to you.

@jsocol
Copy link
Owner

jsocol commented Oct 6, 2015

Looks good! Mind squashing it all down to one commit?

Ensure requests don't get rate limited and don't throw errors when a
memcached instance is unavailable.
@tdhooper tdhooper force-pushed the cope-with-cache-connection-errors branch from 6a541d7 to fa3f0af Compare October 6, 2015 17:07
@tdhooper
Copy link
Contributor Author

Can this be merged now?

jsocol added a commit that referenced this pull request Oct 14, 2015
@jsocol jsocol merged commit 2b67dbc into jsocol:master Oct 14, 2015
@jsocol
Copy link
Owner

jsocol commented Oct 14, 2015

Thanks @tdhooper!

@tdhooper
Copy link
Contributor Author

Thanks, would you mind bumping the version number too?

@jsocol jsocol added this to the 0.7 milestone Oct 15, 2015
@jsocol
Copy link
Owner

jsocol commented Oct 15, 2015

I just added this to the 0.7 milestone, along with the other things that have been merged since 0.6 and a few open issues, all bugs. (I pushed all the new features out to a new 0.8 milestone.)

Before the version bump and release, we want to get those taken care of.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants