Workaround for asyncio.get_event_loop() deprecation

[pepoluan]

What do these changes do?

Implement workaround due to the deprecation in behavior of asyncio.get_event_loop() (which will outright result in error with Python 3.12)

This change of behavior first appeared in Python 3.10: https://docs.python.org/3.10/library/asyncio-eventloop.html#asyncio.get_event_loop

And there has been some discussion on this, among others:

• https://github.com/python/cpython/issues/83710
• https://github.com/python/cpython/issues/93453
• https://github.com/python/cpython/pull/98440

Are there changes in behavior for the user?

Should be minimal.

• If user creates their own event loop and feeds that to aiosmtpd, then nothing will be changed
• If user does not create their own event loop and let aiosmtpd generates one for them, then the behavior totally depends on how get_event_loop_policy() was in effect at time of instantiation.
  • This might or might not be a problem. For asyncio, the behaviour should be similar. I am not sure about the behavior of other implementations (e.g., uvloop)

Related issue number

Closes #353

Checklist

• [x] I think the code is well written
• [x] Unit tests for the changes exist
• [x] tox testenvs have been executed in the following environments:
  • [x] Windows (10): py311-{nocov,cov}
• [x] Documentation reflects the changes

[codecov[bot]]

Codecov Report

Merging #355 (0271497) into master (e4bcd3f) will not change coverage. The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master      #355   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            7         7           
  Lines         1696      1706   +10     
  Branches       310       310           
=========================================
+ Hits          1696      1706   +10     

Impacted Files	Coverage Δ
aiosmtpd/__init__.py	100.00% <100.00%> (ø)
aiosmtpd/handlers.py	100.00% <100.00%> (ø)
aiosmtpd/main.py	100.00% <100.00%> (ø)
aiosmtpd/smtp.py	100.00% <100.00%> (ø)

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

[strongholdmedia]

Is there any sensible write-up as per why (and perhaps when) this policy stuff was introduced with asyncio?

[pepoluan]

Is there any sensible write-up as per why (and perhaps when) this policy stuff was introduced with asyncio?

The deprecation notice first appeared with Python 3.10: https://docs.python.org/3.10/library/asyncio-eventloop.html#asyncio.get_event_loop

There's a discussion about this here: https://github.com/python/cpython/issues/83710

What's being discussed is over my head atm, though. So I'm just taking the 'easy way out' trying to ensure compat by implementing a workaround. And one ugly warning suppression.

Edit: More dicussion here: https://github.com/python/cpython/issues/93453

[pepoluan]

I hope @asvetlov can chime in here. I'm not sure where things are going, and I'd like this PR to be both backward- and forward-compatible with Python >= 3.7

The current changes indeed pass green when fed through the GHCI testing fleet, but it's still making me uneasy.

[waynew]

I've taken a quick glance at this and based on the discussion I'm pretty sure that we need to make a decision within aiosmtpd:

Do we want to take responsibility for the created event loop?

It's looking like historically that's caused problems for folks generally. I think in our case we have two different scenarios:

1. aiosmtpd is the top-level app/process and should go ahead and create the event loop. Based on what I'm seeing on that thread, that should happen via .run().
2. aiosmtpd is used as a library (like I do in orouboros), and the authors should manage the loop.

What that looks like, I think, is that we should use loop or asyncio.get_running_loop().

Comments, questions, rebuttals?

[pepoluan]

I've taken a quick glance at this and based on the discussion I'm pretty sure that we need to make a decision within aiosmtpd:

Do we want to take responsibility for the created event loop?

It's looking like historically that's caused problems for folks generally. I think in our case we have two different scenarios:

1. aiosmtpd is the top-level app/process and should go ahead and create the event loop. Based on what I'm seeing on that thread, that should happen via `.run()`.

2. aiosmtpd is used as a library (like I do in [orouboros](https://github.com/waynew/orouboros)), and the authors should manage the loop.

I'm all for treating aiosmtpd as a pure library, and letting users of aiosmtpd to provide the loop.

However that will introduce some possibly-breaking changes, especially for users that currently uses aiosmtpd as-is, relying on one of the Controllers to spin up an event loop. Like for instance one of the reasons for creation of UnthreadedController which allows user to spin up a different process and let UnthreadedController sets things up in that child process with minimal prepping.

So if we want to to go "pure library" route -- which for the record I'm all for it -- that change will need to wait until at the very least 1.5.0 ... or even 1.6.0 or 2.0.0

This PR is just kind of a band-aid over the underlying problem. At the very least it's meant to provide compat for 3.7 <= Python <= 3.12

[waynew]

It looks like get_running_loop exists in 3.7 https://docs.python.org/3/library/asyncio-eventloop.html#asyncio.get_running_loop

We should be able to use that - and if we want to retain compatibility just catch RuntimeError and start our own event loop.

For the record, I'm entirely OK with having entrypoints (e.g. http.server) that will actually spin up loops, we just need to do that in a responsible way.

[pepoluan]

It looks like get_running_loop exists in 3.7 https://docs.python.org/3/library/asyncio-eventloop.html#asyncio.get_running_loop

We should be able to use that - and if we want to retain compatibility just catch RuntimeError and start our own event loop.

There's a catch though:

This function can only be called from a coroutine or a callback.

Some calls to get_event_loop() took place in the main thread, and there get_running_loop() doesn't work. I forgot exactly what happened when I tried that, but tests got wonky.

[pepoluan]

Aaaaand it seems that the new Warning behavior is getting reverted?

https://github.com/python/cpython/pull/100412

Nevertheless I still want to merge this changes to Cover-Our-Posteriors, so to speak.

Edit: If indeed removal gets rescheduled as Guido informed here we will need to revisit the # pragma: markers, but the logic itself I think is quite sound and adaptable (doesn't try to do things based on Python version)

[waynew]

Well, they're just deferred, and will eventually fail. I only glanced at this, if you'd like me to do a more thorough review I can, otherwise I'm OK with this approach for now

[pepoluan]

Yeah, this PR basically just put a wrapper around asyncio.get_event_loop() to maintain the behavior of the function up to Python 3.9. So just a band-aid to the deeper problems.

I'd say for long term, just like you mentioned, we need to take a step back and implement asyncio more properly. And in the process probably we can also handle Issue #334 by using the 'more modern' way of using asyncio.

[pepoluan]

This PR is not really 'urgent', though I've heard some complaints about the DeprecationWarning.

I'll let this simmer a bit for several more days just to ensure that I haven't overlooked something...