faust-streaming / mode

Python AsyncIO Services
https://faust-streaming.github.io/mode/
Other
43 stars 16 forks source link

Fix passing coroutine objects to `wait()` #24

Closed lqhuang closed 1 year ago

lqhuang commented 1 year ago

Note: Before submitting this pull request, please review our contributing guidelines.

Description

Due to break change introduced in Python 3.11, wait() only accept scheduled tasks.

Changed in version 3.11: Passing coroutine objects to wait() directly is forbidden.

In services.py line 794 of current master, two coroutines are passed into wait() function. So the program exits.

stopped = self._stopped.wait()
crashed = self._crashed.wait()
done, pending = await asyncio.wait(
    [stopped, crashed],
    return_when=asyncio.FIRST_COMPLETED,
    timeout=timeout,
)

This PR fixes #19.

This patch is enough to let mode successfully run under Python 3.11 except passing all test cases. Current tests of mode rely on its self defined AsyncMock which is already not working under py3.11, so there are still a lot of hard work to rewrite test cases to make everything looks good.

wbarnha commented 1 year ago

This patch is enough to let mode successfully run under Python 3.11 except passing all test cases. Current tests of mode relies on its self defined AsyncMock already not working under py3.11,

Very interesting, I'll test this branch with faust-streaming before merging, assuming all test cases above pass for all other versions of Python.

lqhuang commented 1 year ago

@wbarnha Maybe we should stop actions? Something blocked, it already runs 3hrs more. I'm afraid to consume all free time.

lqhuang commented 1 year ago

I see. I removed from asyncio import coroutine which should not show in Python 3.11, so tests for all past Python failed, too. What sad news.

lqhuang commented 1 year ago

@wbarnha Hey, you could review now, I have fixed all tests with new standard library modules.

lqhuang commented 1 year ago

@wbarnha Currently, it fits all versions after Python 3.8. Python 3.7 is lack of test suites AsyncMock, but it should work well too.

wbarnha commented 1 year ago

That's incredible! Fantastic work @lqhuang! I'll see if I can add compatibility for 3.7+3.8, or we could just drop support for those versions.

lqhuang commented 1 year ago

Actually, the incompatibility for both Py3.7 and Py3.8 happens in test codebase. Py3.8 is caused by the nested with syntax.

lqhuang commented 1 year ago

Also, thank you for your efforts in review work!

wbarnha commented 1 year ago

Regarding commit https://github.com/faust-streaming/mode/pull/24/commits/14ca9814d1eda3740be365c339414d7156edb882, I think we need to bring the mocks back because they had specific logic that was hardcoded for faust-streaming unittests...

lqhuang commented 1 year ago

I definitely understand. Sure, you could bring them back, but the key problem is this module also will make faust-streaming unable to run under Python 3.11. I said it would be a hard work to upgrade test cases for both mode and faust.

wbarnha commented 1 year ago

Yeah, I'm starting to remember why you removed the mocks. Upgrading the test cases for Faust will take a while.

lqhuang commented 1 year ago

I also read codes from faust, through I does't use it yet, otherwise I could help to maintain it either. One good thing is you could start from simply replace codes in special pattern. Basically, updating .coro.assert_called_once[_with]() with .assert_awaited_once[_with]() works in most cases. Hope my commits in this PR will give you a hand.

wbarnha commented 1 year ago

Basically, updating .coro.assert_called_once[_with]() with .assert_awaited_once[_with]()

I've observed the same thing! I'll keep trying to go through the unit tests. I've made some progress but there are some quirks I still need to clear up.