PYTHON-4817 Revert import guard on asyncio

[blink1073]

No description provided.

[ShaneHarvey]

I think we still need this guard. Users shouldn't have to forward env vars to make pymongo work in a subprocess.

[blink1073]

That is a general python problem, you need to include certain env variables for it to work.

[blink1073]

Here is the fix we needed: mongodb-labs/drivers-evergreen-tools@14e2fa6

[blink1073]

python/cpython#118234

[ShaneHarvey]

Right but we only suffer from that bug now because we always import asyncio. So we should make all asyncio imports optional in my opinion. Otherwise our users who never even use asyncio could start seeing these errors.

[blink1073]

And I'm saying that it is a whack-a-mole situation and not the way python is meant to be run, as it says in that thread.

[ShaneHarvey]

We can write an import test like this:

with open('asyncio.py', mode='w') as file:
    file.write("raise OSError('mock asyncio import error')")

try:
    import asyncio
except OSError:
    pass
else:
    assert False, "import asyncio succeeded"

import pymongo

[blink1073]

Like I said, this is a whack-a-mole situation and we can't control what CPython has for limitations. I won't be making that test.

[ShaneHarvey]

I disagree. import asyncio being broken should never cause pymongo to fail because that's a regression in pymongo's behavior. PyMongo users shouldn't have to deal with asyncio issues when they're not using asyncio. We already try to do something similar for import ssl and our 3rd party optional imports so there is prior art.

[blink1073]

I still disagree, it isn't broken under normal operating conditions, as expected by CPython. The fact that the previous version of run() worked at all was a brittle accident. If there are minimum requirements to be able to run the standard library functions then we should be able to accept those.

[blink1073]

One thing I should note to clarify: "If env is not None, it must be a mapping that defines the environment variables for the new process; these are used instead of the default behavior of inheriting the current process’ environment."

[Jibola]

@ShaneHarvey @blink1073
I think I'm on the side of removing this type guard. (70:30)

From the vantage point of import ssl, if I understand correctly, that one is less so about importing it when used and more about a tier system of exposed capabilities. Where we'd like for PyOpenSSL to be the primary -- a non stdlib. Then we fallback to the stdlib ssl, and then we give up on SSL support if both manage to fail.

If a stdlib requires env vars to be passed through, I feel that's fair. It needs the system dlls for python to function. Another thought is we could punt on this for now as we deal with other async refinements.

I will say looking at this PR, to make every asyncio import optional would probably leave us with importing asyncio proxied through some lazy util file like with do with openssl. That would be tedious to maintain long term.

# pymongo/asyncio.py

asyncio = None
HAS_ASYNC = True
try:
    import asyncio as _asyncio
    asyncio = _asyncio
except:
    HAS_ASYNC = False

# mongodb/synchronous/example.py

from pymongo.asyncio import asyncio
...

Our pymongo/synchronous also has references to asyncio as is for socket management, so is the idea that we'd only do optional importing in the non /asynchronous subdirectories?

Ultimately, if asyncio (a stdlib) is now an integral part of the codebase the main case for failure is the case of a Windows environment with no env vars is running a subprocess in python (which is already finicky territory asyncio aside), I get the sense we'd be enabling anti-pattern.

[ShaneHarvey]

ssl is intended to be optional no because of pyopenssl but because CPython can be built without the ssl module.

[blink1073]

Right, but asyncio is not considered optional, unless you're running on WASI. My point is that the environment variable limitation is considered a CPython-wide requirement on Windows. The whack-a-mole is spending our time working around what is a hard requirement to run the stdlib, for whichever module manifests the behavior. If we were to account for this, we'd have to add a test that runs on windows that runs PyMongo in a subprocess, specifically removing the system environment variables.

[Jibola]

SGTM!

@blink1073 to point to the CPython ticket around this issue.