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 async support #57

Merged
merged 47 commits into from
May 15, 2023
Merged

Add async support #57

merged 47 commits into from
May 15, 2023

Conversation

pauloromeira
Copy link
Contributor

@pauloromeira pauloromeira commented Apr 11, 2023

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!

@pauloromeira pauloromeira marked this pull request as ready for review April 15, 2023 06:18
@pauloromeira
Copy link
Contributor Author

pauloromeira commented Apr 15, 2023

Hi @fabfuel!

Tests are now passing for: 3.7, 3.8, 3.9, 3.10, 3.11. Maybe it also works for Python 3.6 but unfortunately pytest-asyncio doesn't support 3.6, which is required for pytest to run async tests.

I know you might not like dropping support for versions 2.7, 3.5 and 3.6, but I believe usage can increase with async support. Specially since pybreaker, the most popular alternative, is not supporting async yet.

Two notes:

  1. I slightly changed the fallback functionality for generators to make everything more consistent. Before it was possible to have a fallback function that was not a generator in this case. Well, if the fallback function is called implicitly, I believe it should match the same interface as the decorated function, not just its args. So I changed that, and now for every decorated function: if sync, the fallback must be sync; if generator, the fallback must be a generator; if async, the fallback must be async and if async generator, the fallback must be async generator - I added this information to the README.

  2. The tests are now running, whenever applicable, for every supported function types: sync-function, sync-generator, async-function and async-generator (you can see it by running pytest with the -v flag). This is not only extending generators tests but reusing almost all existing tests for the asynchronous types.

I hope you like and decide to merge it!

Regards,
Paulo

@fabfuel
Copy link
Owner

fabfuel commented Apr 17, 2023

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! 🙂

Copy link
Owner

@fabfuel fabfuel left a 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)
Copy link
Owner

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 🙂

@fabfuel fabfuel merged commit 47ed1b9 into fabfuel:develop May 15, 2023
6 checks passed
@pauloromeira
Copy link
Contributor Author

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! 🎉

@pauloromeira pauloromeira deleted the async-support branch May 17, 2023 22:00
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