`ZEO.asyncio`: switch to `async/await` style #218

navytux · 2023-01-08T18:28:04Z

Second part of Dieter's work from #195 :

Switch to using async/await directly instead of @coroutine/yield
Drop Python 2 and 3.5 support

-------- kirr: ZEO6 is going to support Python3 - see discussion around #195 (comment) . This patch was extracted from #195 .

-------- kirr: Switch to using async def f(): await g() instead of @coroutine def f(): yield g() Remove corresponding runtime support from CoroutineExecutor. Extracted from #195

… with Future`" This reverts commit f85b45b. We stopped support for py2 and trollius. No need to keep on using trollius-related workarounds.

@d-maurer

As we dropped Python2 support, using six is no longer necessary, and we can use Python3 features directly. Suggested by @d-maurer.

-------- kirr: With Python3 we can use builtin unittest.mock instead. This patch was extracted from #195

@d-maurer

-------- As @d-maurer says unittest.mock in Python3.5 lacks some functionality that we use, e.g. assert_called_once. Python 3.5 was marked as EOL in 2017, and it stopped receiving security updates in 2020. It should be safe to drop support for it.

@d-maurer

@d-maurer says that manuel is not used at all. Indeed it was there in setup.py from 823dad3 (Stripped to just ZEO) - the time when ZODB repository was split into ZODB, ZEO, BTrees and Persistent, and in ZEO manuel occurrence remained just as leftover after that split.

Msgpack egg was pinned to be < 1 in 503dccb (prevent deprecation warning and move to latest msgpack supporting Python 2). We no longer support Python2, thus we no longer need to pin msgpack to the version that still supports it.

-------- kirr: Python3 allows to use just super() without arguments. Extracted from #195

-------- kirr: We dropped support for py2 and trollius, and now plain `import asyncio` always works. Extracted from #195

-------- kirr: Extracted from #195

…ility and efficiency use `nonlocal` (instead of mutable wrapping) for shared variables -------- kirr: no need to change _smp.pyx since for cdef classes self.var works efficiently without dict lookup. Extracted from #195

-------- kirr: On python3 there are no old-style and new-style classes. All classes are new style and object is always used as default base class. Extracted from #195

…lts just ok -------- kirr: Extracted from #195

-------- kirr: Extracted from #195

navytux · 2023-01-08T18:50:04Z

It is a question whether we want to merge this - from #195 (comment):

the second part drops Python2 support and switches from async/await-style
coroutines being implemented with @coroutine/yield syntax to those
coroutines implemented with async/await syntax directly. Due to the way
how part 1 is organized this change is small. It is represented by
7f574ecd patch and further
changes that drop py2 support otherwise in knext+py3 branch and
ZEO.asyncio: switch to async/await style #218 pull-request.

In Drop Python2 support (WIP: do not merge before #208 or #195) #200 (comment) you
said that "dropping Python 2 support gives no gain by itself. Only if
gainfull things (I think of ZEO.asyncio: switch to async/await style #195) require dropping Python 2 support, I would
merge Drop Python2 support (WIP: do not merge before #208 or #195) #200." So from this point of view, if maybe we are ok to keep our
async/await-style coroutines being implemented with similar
@coroutine/yield syntax, there is no real need to merge this and to drop
py2 support.

d-maurer · 2023-01-08T19:00:51Z

Kirill Smelkov wrote at 2023-1-8 10:50 -0800:

... In #200 (comment) you said that _"dropping Python 2 support gives no gain by itself. Only if gainfull things (I think of #195) require dropping Python 2 support, I would merge #200."_ So from this point of view, if maybe we are ok to keep our `async`/`await`-style coroutines being implemented with similar ***@***.***`/`yield` syntax, there is no real need to merge this and to drop py2 support.

I got the feeling that all `zopefoundation` packages will drop Python 2 support the next time, they are reconfigured with the `meta` tools. Thus, after you have merged the Py2/Py3 changes, you should likely make a 5.x branch.

navytux · 2023-01-08T19:02:14Z

Thanks, I see.

dataflake · 2023-01-09T06:29:27Z

We are in the slow process of dropping support for all Python versions up to and including Python 3.6. Many dependencies have already done that and the pain of supporting those old Python versions is now higher than any gain.

navytux · 2023-01-09T08:18:30Z

I see, Jens. Thanks for clarifying. So from this point of view it indeed makes sense to want to merge this PR.

dataflake · 2023-01-09T10:36:23Z

IMHO you should also remove Python 3.6 support before merging.

* knext: asyncio: Remove support for calling ConcurrentTask.cancel from non-IO threads asyncio: Don't reset current loop after loop_run_* completes fix discrepancy between `futures.py` and `_futures.pyx` (reported by @navitux) implement `ConcurrentTask.cancel` based on the event loop rather than the thread id asyncio.futures: Simplify ConcurrentTask.cancel() tests: Remove explicit `def test_suite` where test suite definition is trivial tests: Remove explicit calls to unittest.main Remove support for 'python setup.py test' fixup! asyncio.futures: Implement cancellation thoroughly fixup! asyncio.client: make `prefetch` more reliable fixup! asyncio.futures: Add own coroutine executor fixup! asyncio.futures: Implement cancellation thoroughly

navytux · 2023-01-13T19:17:30Z

IMHO you should also remove Python 3.6 support before merging.

@dataflake, would you please clarify about why? Currently supporting py3.6 does not add big extra cost.

dataflake · 2023-01-13T20:44:32Z

It is EOF and we're removing support on all packages we touch in the zopefoundatiom organization.

* knext: fixup! asyncio.futures: Mimic async/await with plain yield and @coroutine decorator

@dataflake

@dataflake says at #218 (comment) : IMHO you should also remove Python 3.6 support before merging. ... It is EOF and we're removing support on all packages we touch in the zopefoundatiom organization.

navytux · 2023-01-19T13:16:05Z

Thanks, Jens. I removed it in b4507e55.

navytux · 2023-01-19T13:18:37Z

Btw, Dieter, any idea about https://github.com/zopefoundation/ZEO/actions/runs/3958283359/jobs/6779667424 ?

d-maurer · 2023-01-19T22:07:18Z

I think after we removed that extra py2 supporting code from CoroutineExecutor,
the difference you point out should not be a blocker. I believe it is better to
use our own Futures and Tasks throughout uniformly - so that they are all
properly tested and we are sure that it works all ok.

Our tasks not only execute callbacks immediately, they ignore also the task context (used e.g. for context variables). We know that our coroutines make no use of the task context; however, a priori, we know nothing about the "create connection coroutine". In my opinion, it would therefore be safer to use loop.create_task at least for the "create connection coroutine" as this guarantees a task which provides all of the standard asyncio.Task functionality (including full context support).

@d-maurer

… of our Task Even though everything seems to be working with our Task implementation @d-maurer suggests to use native Task implementation for coroutine code implemented outside of ZEO because this way it is potentially safer: #218 (comment) -> Let it be this way.

navytux · 2023-01-19T22:37:27Z

Dieter, thanks for feedback.

Ok, let it be this way. I've changed ._connecting to be created via
loop.create_task as you suggest: 63429391.

Kirill

d-maurer · 2023-01-20T08:21:41Z

Btw, Dieter, any idea about https://github.com/zopefoundation/ZEO/actions/runs/3958283359/jobs/6779667424 ?

This is a race condition caused by a buggy test:
the test sets up a ClientStorage for storage 2 -- which the server does not have. It then closes the storage and checks the log records. The test succeeds when the server response about the unknown storage has not yet been processed (this is typically the case); otherwise, the log records associated with this processing let the test fail.

The log record examination exists primarily to help in the analysis of earlier problems with the closing logic: in some cases, the close deadlocked. To work around this, I introduced a timeout into the closing and the generation of a log message when it terminates the closing process. The log record examination should detect a corresponding log record but actually does more. I suggest to replace it with

    >>> def check_log_records(records):
    ...     records = [r.message for r in records]
    ...     return any("bug" in r for r in records) and records else []

navytux · 2023-01-20T08:52:43Z

Dieter, thanks for the approval and for explaining what is going on with check_log_records.

I suggest to replace it with

>>> def check_log_records(records):
    ...     records = [r.message for r in records]
    ...     return any("bug" in r for r in records) and records else []

I agree. Would you please do this, probably with also putting a comment regarding the race you describe? I mean this is your test and your code, and it was you to analyze what is going on here. It looks logical for you to make the fixing commit.

@dataflake, are you also ok with merging this PR after we do the above?

d-maurer · 2023-01-20T09:42:50Z

Kirill Smelkov wrote at 2023-1-20 00:52 -0800:

... I agree. Would you please do this ...

Ok -- when I have finished my current project.

navytux · 2023-01-20T10:28:14Z

Thanks.

navytux · 2023-01-23T08:49:20Z

Dieter, thanks for the fixups. Do I understand it right that this PR should be ok to be merged now?

navytux · 2023-01-23T08:50:20Z

@dataflake, are you also ok with merging this PR after we do the above?

Jens, we seem to be done. Do you agree with merging this?

dataflake · 2023-01-23T09:27:20Z

Yes you can go ahead and merge

navytux · 2023-01-23T11:05:11Z

Dieter, Jens, thanks for feedback and approvals. I've merged this via 59de2de3.

Using the occassion I have a question regarding 5.x branch: now since hereby PR is merged, I want to pick some fixes and improvements from master to 5.x . For example I would like to pick e.g. d08ac83, removal of ZEO/zhash.py, 3ca79a6, changing deprecated threading API to up-to-date ones (e.g. setDaemon(True) -> .daemon = True), etc. Do I need to open PR for such backports, or is it ok if I do it in 5.x directly by myself?

dataflake · 2023-01-23T11:08:06Z

I think it's fine if you do this by yourself.

@dataflake

This file was used for py2.6 compatibility which is long gone and nothing imports ZEO.hash this days anymore. Confirmed by @dataflake: #218 (comment) (cherry-picked from b4507e5)

navytux · 2023-01-23T12:36:20Z

Thanks, Jens. I picked up the following patches to 5.x for now:

I did not picked change of % to .format because sometimes you changed to .format and sometimes to using f-strings. And even though .format is supported on both py2 and py3, f-strings are supported only on py3. I wanted to keep the diff in between 5.x and master minimal for easier tracking, but regarding strings I'm not sure it makes sense to pick up only partial change to .format.

Hope my backports are ok.

dataflake · 2023-01-23T12:39:07Z

You do not need to verify every detail with me. I don't have much interest in the ZEO 5.x branch. My goal is to move the main "consumer" of ZEO, Zope, to ZEO 6 as soon as the first release is there.

navytux · 2023-01-23T12:39:36Z

I see. Thanks for feedback.

d-maurer and others added 15 commits January 8, 2023 20:13

Drop Python2 support

3ca4d5d

-------- kirr: ZEO6 is going to support Python3 - see discussion around #195 (comment) . This patch was extracted from #195 .

ZEO.asyncio: switch to async/await style

7f574ec

-------- kirr: Switch to using async def f(): await g() instead of @coroutine def f(): yield g() Remove corresponding runtime support from CoroutineExecutor. Extracted from #195

Revert "asyncio: Fix occasional `ValueError: loop argument must agree…

d8673b5

… with Future`" This reverts commit f85b45b. We stopped support for py2 and trollius. No need to keep on using trollius-related workarounds.

Drop use of six

d0f0709

As we dropped Python2 support, using six is no longer necessary, and we can use Python3 features directly. Suggested by @d-maurer.

Drop use of mock egg

3ea9bf3

-------- kirr: With Python3 we can use builtin unittest.mock instead. This patch was extracted from #195

drop Python 3.5 support

3458abc

-------- As @d-maurer says unittest.mock in Python3.5 lacks some functionality that we use, e.g. assert_called_once. Python 3.5 was marked as EOL in 2017, and it stopped receiving security updates in 2020. It should be safe to drop support for it.

No longer pin msgpack < 1

1787d34

Msgpack egg was pinned to be < 1 in 503dccb (prevent deprecation warning and move to latest msgpack supporting Python 2). We no longer support Python2, thus we no longer need to pin msgpack to the version that still supports it.

*: Replace super(Type, self) -> super()

41288d0

-------- kirr: Python3 allows to use just super() without arguments. Extracted from #195

ZEO.asyncio: No need to import asyncio through compat

6d9d2c4

-------- kirr: We dropped support for py2 and trollius, and now plain `import asyncio` always works. Extracted from #195

*: No need to import threading-related bits through compat

2c9c7eb

-------- kirr: Extracted from #195

*: No need to use class X(object)

fceb295

-------- kirr: On python3 there are no old-style and new-style classes. All classes are new style and object is always used as default base class. Extracted from #195

asyncio.tests: Python3 handles *argv and keyword arguments with defau…

9cff397

…lts just ok -------- kirr: Extracted from #195

ZEO6 α1

e17cd2a

-------- kirr: Extracted from #195

This was referenced Jan 8, 2023

Drop Python2 support (WIP: do not merge before #208 or #195) #200

Closed

ZEO.asyncio: switch to async/await style #195

Closed

navytux requested a review from d-maurer January 8, 2023 19:00

navytux mentioned this pull request Jan 10, 2023

asyncio.client improvements #217

Merged

navytux added 2 commits January 14, 2023 20:57

Merge branch 'knext' into knext+kpy3

fd030ef

* knext: fixup! asyncio.futures: Mimic async/await with plain yield and @coroutine decorator

Drop support for Python3.6

7ecc6a3

@dataflake says at #218 (comment) : IMHO you should also remove Python 3.6 support before merging. ... It is EOF and we're removing support on all packages we touch in the zopefoundatiom organization.

- small fixes from eyeballing the PR diff [ci skip]

02ceb12

add comment about the special task use for the connect coroutine

e3b8658

d-maurer self-requested a review January 20, 2023 08:22

d-maurer approved these changes Jan 20, 2023

View reviewed changes

d-maurer added 2 commits January 23, 2023 08:20

make test client-config.test more robust against application/ZEO race

d08ac83

better highlight the limitations of ZEO Futures and Tasks

3ca79a6

navytux requested a review from d-maurer January 23, 2023 08:49

d-maurer approved these changes Jan 23, 2023

View reviewed changes

dataflake approved these changes Jan 23, 2023

View reviewed changes

navytux merged commit 59de2de into master Jan 23, 2023

navytux deleted the knext+kpy3 branch January 23, 2023 10:52

navytux added a commit that referenced this pull request Jan 23, 2023

[ZEO5] Remove ZEO/hash.py

421996d

This file was used for py2.6 compatibility which is long gone and nothing imports ZEO.hash this days anymore. Confirmed by @dataflake: #218 (comment) (cherry-picked from b4507e5)

navytux mentioned this pull request Mar 30, 2023

[ZEO5] asyncio.client: Fix connect logic wrt IO loop startup #227

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`ZEO.asyncio`: switch to `async/await` style #218

`ZEO.asyncio`: switch to `async/await` style #218

navytux commented Jan 8, 2023

navytux commented Jan 8, 2023

d-maurer commented Jan 8, 2023 via email

navytux commented Jan 8, 2023

dataflake commented Jan 9, 2023

navytux commented Jan 9, 2023

dataflake commented Jan 9, 2023

navytux commented Jan 13, 2023

dataflake commented Jan 13, 2023

navytux commented Jan 19, 2023

navytux commented Jan 19, 2023

d-maurer commented Jan 19, 2023

navytux commented Jan 19, 2023

d-maurer commented Jan 20, 2023

navytux commented Jan 20, 2023

d-maurer commented Jan 20, 2023 via email

navytux commented Jan 20, 2023

navytux commented Jan 23, 2023

navytux commented Jan 23, 2023

dataflake commented Jan 23, 2023

navytux commented Jan 23, 2023

dataflake commented Jan 23, 2023

navytux commented Jan 23, 2023

dataflake commented Jan 23, 2023

navytux commented Jan 23, 2023

ZEO.asyncio: switch to async/await style #218

ZEO.asyncio: switch to async/await style #218

Conversation

navytux commented Jan 8, 2023

navytux commented Jan 8, 2023

d-maurer commented Jan 8, 2023 via email

navytux commented Jan 8, 2023

dataflake commented Jan 9, 2023

navytux commented Jan 9, 2023

dataflake commented Jan 9, 2023

navytux commented Jan 13, 2023

dataflake commented Jan 13, 2023

navytux commented Jan 19, 2023

navytux commented Jan 19, 2023

d-maurer commented Jan 19, 2023

navytux commented Jan 19, 2023

d-maurer commented Jan 20, 2023

navytux commented Jan 20, 2023

d-maurer commented Jan 20, 2023 via email

navytux commented Jan 20, 2023

navytux commented Jan 23, 2023

navytux commented Jan 23, 2023

dataflake commented Jan 23, 2023

navytux commented Jan 23, 2023

dataflake commented Jan 23, 2023

navytux commented Jan 23, 2023

dataflake commented Jan 23, 2023

navytux commented Jan 23, 2023

`ZEO.asyncio`: switch to `async/await` style #218

`ZEO.asyncio`: switch to `async/await` style #218