-
-
Notifications
You must be signed in to change notification settings - Fork 50
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 async support #57
Conversation
ccb91a4
to
9715fb3
Compare
b24bcbb
to
c500445
Compare
d884230
to
60a1a20
Compare
60a1a20
to
3977194
Compare
Hi @fabfuel! Tests are now passing for: I know you might not like dropping support for versions Two notes:
I hope you like and decide to merge it! Regards, |
e419dd1
to
db031dc
Compare
Thanks a lot @pauloromeira! Sorry, I'm a bit behind as i had no time to fully review yet, but what I saw so far looks really good. I try to find time to also test it as soon as possible! 🙂 |
d998af0
to
a81a5b2
Compare
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.
Sorry for the long delay @pauloromeira, I needed to prepare for parental leave and had very busy last weeks while currently not working in the Python universe, so I needed some time to get into the topic again. I also did not use Python async functions in production yet, so it's awesome, that you took care for adding the functionality 🙏
Thanks a lot for your PR, it's really well done. I highly appreciate your effort and the way how you integrated the async functionality into the existing library in an unobtrusive way (=without rewriting everything 😉).
The tests have been quite rewritten though, so I needed some time to test myself and get back into it. But finally I reviewed it and I'm happy to merge.
I will create version 2.0, I think this new ability deserves a new major version, especially also as we drop Python2 support.
Thanks a lot @pauloromeira!
|
||
@pytest.fixture | ||
def circuit_success(function): | ||
return CircuitBreaker()(function) |
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.
Oh, that's a nice syntax, I did not know 🙂
You're welcome, @fabfuel! Thank you for dedicating your time and effort to reviewing it. I appreciate your decision to proceed with the merge, despite the drop of support for Python 2. Wishing you all the best in your journey of parenthood! 🎉 |
Implement: #4
Hi @fabfuel ,
First of all, congrats for the lib!
This PR adds
async
support to it, but this change will drop python 2 support and some python 3 early versions.I'm opening it now to kindly ask you some help for making this available for the majority of python versions and change setup / gh actions accordingly.
Thanks in advance!