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

Add optional support for calling Tornado coroutines #18

Merged
merged 4 commits into from
Jun 21, 2017

Conversation

jeffrand
Copy link
Contributor

I'm a big fan of your library and I've been using my own fork over here at SeatGeek, so I figured I'd contribute. We use Tornado as our webserver and use this code to circuit break asynchronous code.

This change adds optional support for Tornado coroutines. This should be a backwards compatible change.

I did this by:

  • Modifying CircuitBreaker.__call__ to optionally take a keyword argument __pybreaker_call_async, which will call CircuitBreaker.call_async if True. The keyword argument is sufficiently verbose to hopefully avoid collision with existing keyword arguments
  • Adding CircuitBreaker.call_async
  • Adding CircuitBreakerState.call_async
  • Duplicating the import pattern used by HAS_REDIS_SUPPORT

This also has tests that cover the parts that I added. It also fixes a bug if redis is not present.

jeffrand added 4 commits June 15, 2017 16:14
Signed-off-by: jeffrand <jeffr@seatgeek.com>
Signed-off-by: jeffrand <jeffr@seatgeek.com>
Signed-off-by: jeffrand <jeffr@seatgeek.com>
… arguments

Signed-off-by: jeffrand <jeffr@seatgeek.com>
Copy link
Owner

@danielfm danielfm left a comment

Choose a reason for hiding this comment

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

LGTM 💯

@danielfm danielfm merged commit 82c56b5 into danielfm:master Jun 21, 2017
@danielfm
Copy link
Owner

Totally forgot to mention this, but can you please update the README with usage instructions?

@danielfm danielfm mentioned this pull request Jun 21, 2017
@jeffrand
Copy link
Contributor Author

jeffrand commented Jun 21, 2017 via email

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.

2 participants