Remove manual contextvar copy logic

[Kludex]

anyio 3.4.0 copies the context already: agronholm/anyio#390

There's no need for limiting the range just for that logic, but that's something we can clean up (now or in the future).

[adriangb]

I ran this change on anyio==3.0.0 and nothing broke. So it seems like we don't have a test for this (probably the code is covered, but not the logic).

So maybe it's worth adding a quick test before merging this to be sure things work as expected?

def test_accessing_context_from_threaded_sync_endpoint() -> None:

    ctxvar: ContextVar[bytes]  = ContextVar("ctxvar")
    ctxvar.set(b"data")

    def endpoint(request: Request) -> Response:
        return Response(ctxvar.get())
    
    app = Starlette(routes=[Route("/", endpoint)])
    client = TestClient(app)
    
    resp = client.get("/")
    assert resp.content == b"data"

[tomchristie]

Ooohhh.... nice bit of tidying.
I do like tidying.

[Kludex]

I've just noticed that this doesn't work for Python 3.6, so I can expect a pipeline failure on the following commit.

EDIT: To be fair, it didn't work for Python 3.6 before... So not a relevant comment.

[Kludex]

Well... That was unexpected...

[adriangb]

Maybe import contextvars within the test and otherwise skip the test?

def test_accessing_context_from_threaded_sync_endpoint() -> None:
    try:
        from contextvars import ContextVar
    except ImportError:
        pytest.skip('test requires contextvars package to be available')

    ctxvar: ContextVar[bytes]  = ContextVar("ctxvar")
    ctxvar.set(b"data")

    def endpoint(request: Request) -> Response:
        return Response(ctxvar.get())
    
    app = Starlette(routes=[Route("/", endpoint)])
    client = TestClient(app)
    
    resp = client.get("/")
    assert resp.content == b"data"

[adriangb]

Well... That was unexpected...

Maybe because we require the back port on < 3.7?

starlette/setup.py

Line 43 in 5a5a5c3

"contextlib2 >= 21.6.0; python_version < '3.7'",

[Kludex]

Well... That was unexpected...

Maybe because we require the back port on < 3.7?

starlette/setup.py

Line 43 in 5a5a5c3

"contextlib2 >= 21.6.0; python_version < '3.7'",

That's for contextlib, not for contextvars 🤔

[Kludex]

AnyIO installs https://pypi.org/project/contextvars/ for Python 3.6 :P

[adriangb]

That's for contextlib, not for contextvars 🤔

🤦 my bad

AnyIO installs https://pypi.org/project/contextvars/ for Python 3.6 :P

So I guess we're good then? The test demonstrates that things work, and we know why they work, so I don't see any issue right?

[Kludex]

Yep.

The only "issue" is restricting anyio's possible versions. It's also important to say that anyio is working on the 4.0 release.

Besides that, everything is cool.

[Kludex]

Since the range of anyio will be >=3.4.0,<5.0, we can merge this 👍

[adriangb]

Awesome, thank you @Kludex !