Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test client usage question: how to trigger lifespan startup and shutdown ? #350

Closed
euri10 opened this issue Sep 16, 2019 · 19 comments
Closed

Comments

@euri10
Copy link
Member

euri10 commented Sep 16, 2019

I have an issue switching from the TestClient from async_asgi_testclient to the httpx.AsyncClient : the startup and shutdown events of my app aren't triggered by the latter.

It's probably a bad usage on my end, but if anyone could pinpoint what I'm doing wrong it would be of tremendous help,

I wrote a small set of 2 tests below to better describe the situation, the one with httpx.AsyncClient fails while the other passes fine:

import logging

import pytest
from async_asgi_testclient import TestClient
from httpx import AsyncClient
from starlette.applications import Starlette
from starlette.requests import Request
from starlette.responses import JSONResponse

logger = logging.getLogger(__name__)
logger.setLevel(logging.DEBUG)

app = Starlette()


@app.on_event("startup")
async def startup():
    logger.debug("starting the app")


@app.on_event("shutdown")
async def shutdown():
    logger.debug("shutdown the app")


@app.route("/")
def homepage(request: Request):
    logger.debug("HOME")
    return JSONResponse({"hello": "world"})


@pytest.fixture
async def asyncasgitestclient():
    yield TestClient(app)


@pytest.fixture
async def httpxclient():
    yield AsyncClient(app=app, base_url="https://domain.tld")


@pytest.mark.asyncio
async def test_homepage_with_httpxclient(httpxclient, caplog):
    async with httpxclient:
        url = app.url_path_for("homepage")
        resp = await httpxclient.get(url)
        assert resp.status_code == 200
        assert resp.json() == {"hello": "world"}
    logger.debug(caplog.messages)
    assert all(
        x in caplog.messages for x in ["starting the app", "HOME", "shutdown the app"]
    )


@pytest.mark.asyncio
async def test_homepage_with_async_asgi_testclient(asyncasgitestclient, caplog):
    async with asyncasgitestclient:
        url = app.url_path_for("homepage")
        resp = await asyncasgitestclient.get(url)
        assert resp.status_code == 200
        assert resp.json() == {"hello": "world"}
    logger.debug(caplog.messages)
    assert all(
        x in caplog.messages for x in ["starting the app", "HOME", "shutdown the app"]
    )
@florimondmanca
Copy link
Member

florimondmanca commented Sep 16, 2019

Hi @euri10, I think what you are seeing is the intended behavior: the HTTPX client isn’t tied to your Starlette app, so if you want to create a client and trigger event handlers on the app, you must enter both context managers:

async with httpxclient, asyncasgitestclient:
    ...

Edit: actually Insee you do pass the app to the AsyncClient, so I’m not sure anymore whether this is a bug or the intended behavior. Still, I think the code above is a valid workaround. :) I’ll let others make the call on whether this should be considered a bug on the ASGI integration.

@sethmlarson
Copy link
Contributor

cc @tomchristie

@euri10
Copy link
Member Author

euri10 commented Sep 17, 2019

Edit: actually Insee you do pass the app to the AsyncClient, so I’m not sure anymore whether this is a bug or the intended behavior. Still, I think the code above is a valid workaround. :) I’ll let others make the call on whether this should be considered a bug on the ASGI integration.

neat workaround indeed, however it feels awkward to have to use 2 testclients ! is it worth opening a feature request on Starlette to get async support for the TestClient there ?

@florimondmanca
Copy link
Member

florimondmanca commented Sep 17, 2019

it feels awkward to have to use 2 testclients !

I think it can make sense in terms of separation of concerns: should it be HTTPX's concern to startup/shutdown the ASGI app it was given? I guess the one question we need to answer is: are there situations in which we'd be given an app that hasn't received the lifespan.startup and lifespan.shutdown? If not, then we could consider sending lifespan events when starting/closing the client.

Actually, thinking about this in terms of allowing users to switch to AsyncClient from Starlette's TestClient (with the end goal that TestClient could be removed from Starlette entirely), the AsyncClient not implementing the lifespan protocol is definitely a bug.

is it worth opening a feature request on Starlette to get async support for the TestClient there ?

I remembered there was already one here :) encode/starlette#104

Regardless of the outcome of this issue, I'd be in favor of documenting how HTTPX deals with application lifespan in the docs: Calling into Python web apps.

@tomchristie
Copy link
Member

Out of scope for HTTPX.

Either Starlette's test client could use HTTPX, but could additionally provide lifespan support, or Starlette should explcitly include a lifespan controller for use in test cases, that's properly independent of the test client.

I think the later case probably makes the most sense, but either way it's independent of httpx (Although our ASGIdispatch docs could end up linking to or discussing it very briefly)

@tomchristie
Copy link
Member

Actually, thinking about this in terms of allowing users to switch to AsyncClient from Starlette's TestClient (with the end goal that TestClient could be removed from Starlette entirely), the AsyncClient not implementing the lifespan protocol is definitely a bug.

I think there's a case to be made there, but I'm quite keen on treating it as an entierly seperate issue. (Users are also free to deal with application setup/teardown in test cases in more explicit ways rather than relying on ASGI's lifespan messaging)

@florimondmanca
Copy link
Member

florimondmanca commented Sep 27, 2019

While I understand the need for separating concerns, I’ll say I’m a bit frustrated that this has to be deferred to Starlette.

The lifespan protocol is part of the ASGI standard — having to tell users to install and use on a particular framework for sending lifespan events to their app doesn’t feel right to me.

Maybe there’s room for an agnostic lifespan app released as a separate library, or as part of asgiref? That would need to deal with async library agnosticity too though, which kind of itches me, but oh well.

(Another thing feeding this intuition is that @euri10 asked about async_asgi_testclient, not Starlette.)

@tomchristie
Copy link
Member

Maybe there’s room for an agnostic lifespan app released as a separate library

Oh absolutely yes, although it also ought to be small enough to be a snippet. (Plus in any case Starlette is deliberately a zero-hard-dependency install in order to allow it to be used either full-framework or bits-of-ASGI-toolkit.)

It's just not at all obvious to me that "startup/shutdown the app" should have anything at all to do with HTTPX's "send some HTTP requests to the app" whatsoever.

As an example, you might need a lifespan context to be independant of the HTTP client lifetime, such as instantiating two different clients, one which represents a logged-in user, with associated session credentials, and on representing some other user session, and making requests with both.

@florimondmanca
Copy link
Member

Yup, I’m sold on the idea of decoupling the two, definitely.

@sethmlarson
Copy link
Contributor

Dropping in just to say that it's a bummer that a full ASGI use-case can't be solved with just HTTPX. IMO this is one of our defining features as an HTTP client in the ecosystem.

@tomchristie
Copy link
Member

tomchristie commented Sep 27, 2019

I don't think it's an issue. Yeah we want to resolve it (not in this package tho). And yeah we may want to point to that from the docs, too, so that we're pointing out a full solution.

It's just that it happens that ASGI is more complete than WSGI in this case in that it has proper startup/shutdown events in place rather than just "probably run some stuff on the first HTTP request we see".

There's nothing to stop users from running whatever startup/shutdown they need to around test cases, which they'd need to eg. be doing in a WSGI case anyways. We're just hitting a very tiny bump of "havn't quite filled in this gap".

I might try to take a little look at a sketching it out this afternoon, since it's a nice little stand-alone code snippet.

@florimondmanca
Copy link
Member

Okay so, I created https://github.com/florimondmanca/asgi-lifespan to experiment on this. It's mostly empty for now but I'll add LifespanContext based on the implementation in #352, and I plan to have LifespanMiddleware be mostly ripped out from Starlette.

Any thoughts on the proposed API there?

@tomchristie
Copy link
Member

@florimondmanca
Copy link
Member

florimondmanca commented Sep 29, 2019

@euri10 Here's your initial code snippet rearranged to use asgi-lifespan. Right now you need to use @pytest.mark.anyio instead of @pytest.mark.asyncio because of agronholm/anyio#74, but this should get sorted out soon.

import logging
import pytest
from asgi_lifespan import LifespanManager
from httpx import AsyncClient
from starlette.applications import Starlette
from starlette.requests import Request
from starlette.responses import JSONResponse

logger = logging.getLogger(__name__)
logger.setLevel(logging.DEBUG)

app = Starlette()


@app.on_event("startup")
async def startup():
    logger.debug("starting the app")


@app.on_event("shutdown")
async def shutdown():
    logger.debug("shutdown the app")


@app.route("/")
def homepage(request: Request):
    logger.debug("HOME")
    return JSONResponse({"hello": "world"})


@pytest.fixture
async def httpxclient(caplog):
    async with LifespanManager(app):
        async with AsyncClient(app=app, base_url="https://domain.tld") as client:
            yield client
    logger.debug(caplog.messages)
    assert all(
        x in caplog.messages for x in ["starting the app", "HOME", "shutdown the app"]
    )


@pytest.mark.anyio
async def test_homepage_with_httpxclient(httpxclient):
    url = app.url_path_for("homepage")
    resp = await httpxclient.get(url)
    assert resp.status_code == 200
    assert resp.json() == {"hello": "world"}

@kernc
Copy link

kernc commented Dec 30, 2020

And yeah we may want to point to that from the docs, too, so that we're pointing out a full solution.

Just like to point out there's still no mention of asgi_lifespan.LifespanManager (or an alternative) in the docs here: https://www.python-httpx.org/async/#calling-into-python-web-apps

@florimondmanca
Copy link
Member

@kernc Thanks, I logged #1441 so that we're properly tracking this, and so that's visible to anyone up for contributing. :)

@ammarsaf
Copy link

ammarsaf commented Jun 11, 2023

Using lifespan on APIRoute from FastAPI

I am not using Starlette, but I am using FastAPI here (the wrapper of Starlette). I tried to apply lifespan event to APIRoute from FastAPI, but it did not support for route, even though the route class has attribute lifespan. At the moment, I only can apply the lifespan to app = FastAPI(lifespan=lifespan) instead of the router, by following FastAPI documentation here. I found this is the closes discussion that existed at the moment.

Has anyone tried this before? @florimondmanca

@zanieb
Copy link
Contributor

zanieb commented Jun 13, 2023

@Ammar-Azman Hey! This issue is for adding support for calling lifespan events from a HTTPX client not defining them. Please open a discussion on FastAPI instead. Make sure you read the documentation on lifespan events first — I believe they are always application not router scoped.

@ammarsaf
Copy link

@Ammar-Azman Hey! This issue is for adding support for calling lifespan events from a HTTPX client not defining them. Please open a discussion on FastAPI instead. Make sure you read the documentation on lifespan events first — I believe they are always application not router scoped.

Thanks for the response. I have open the discussion on FastAPI repo and the dev said router is not supported with lifespan, at the moment. Would you like me to delete this conversation?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants