bpo-36373: Fix deprecation warnings#15889
Conversation
|
@tirkarthi that's what I mean |
| async def _asyncioLoopRunner(self): | ||
| queue = self._asyncioCallsQueue | ||
| async def _asyncioLoopRunner(self, fut): | ||
| self._asyncioCallsQueue = queue = asyncio.Queue() |
There was a problem hiding this comment.
Just curious why we moved it here. Is there a difference in constructing it in _setupAsyncioLoop itself without passing loop and having it here?
There was a problem hiding this comment.
Well, it is possible to just drop the loop parameter from Queue construction because we installed the default event loop a few lines above but I did two steps further.
Instantiation on asyncio loop coupled objects from an async function is more idiomatic, it always grabs a currently executed loop (the loop which is used to run a coroutine).
It prevents things like this:
import asyncio
queue = asyncio.Queue()
asyncio.run(...) # a code that works with queue
In the example queue variable is instantiated with the default asyncio loop but run() creates a new loop version. So the queue doesn't work and hangs eventually because a future created by one loop cannot be awaited by another one.
In Python 3.9 I have a plan to add another deprecation warning if not self._loop.is_running() for queues and locks and eventually replace get_event_loop() with get_running_loop() as a final fix.
There was a problem hiding this comment.
LGTM. As I understand previously we were always passing self._loop which would always be initialized with a loop object and would mean we are passing the loop value to the other callers though the initial caller had loop=None only. This looks more correct and simple to me given asyncio.Queue() is a valid call. Thanks Andrew.
|
I thought we were going to be more subtle about this. We have two scenarios:
The (1) approach is how people were used to writing asyncio programs before The (2) approach is how we want people to write asyncio programs. There's a subtle difference between things like If we remove the @asvetlov thoughts? |
1st1
left a comment
There was a problem hiding this comment.
Please don't merge this until we discuss it.
|
When you're done making the requested changes, leave the comment: |
|
On Python 3.6 where Also, please read comment above. In aiohttp we forbade the creation of |
|
Alright. |
|
Thanks @asvetlov for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8. |
|
GH-15896 is a backport of this pull request to the 3.8 branch. |
|
Thanks @asvetlov for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8. |
|
Sorry @asvetlov, I had trouble checking out the |
|
GH-15901 is a backport of this pull request to the 3.8 branch. |
https://bugs.python.org/issue36373 (cherry picked from commit 7264e92) Co-authored-by: Andrew Svetlov <andrew.svetlov@gmail.com>
https://bugs.python.org/issue36373 (cherry picked from commit 7264e92) Co-authored-by: Andrew Svetlov <andrew.svetlov@gmail.com>
|
Is there a drop in replacement for code that was previously using the |
|
I had the same issue and found this workaround. Forcing the introduction of factory functions counts as progress towards Java, I guess... |
|
I ended up here after following deprecation warning -> git blame -> ticket -> comments. I have a case of an asynchronous loop running on another thread already, and a synchronous program that creates tasks to run on it. Basically like background workers. Trying to go a generic way here. Maybe they would be some existing helpers but I did not find it yet. |
https://bugs.python.org/issue36373
Automerge-Triggered-By: @asvetlov