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

Async callbacks feature request #43

Closed
calebho opened this issue Aug 9, 2021 · 6 comments
Closed

Async callbacks feature request #43

calebho opened this issue Aug 9, 2021 · 6 comments
Labels
question Further information is requested

Comments

@calebho
Copy link
Contributor

calebho commented Aug 9, 2021

I have a potential use case for an async callback which was originally mentioned in #38. I have the following (pseudo-)code which I want to test:

async def foo():
    async with httpx.AsyncClient() as c:
        with trio.move_on_after(timeout) as cs:
            await c.post(endpoint, json=payload)
        if cs.cancelled_caught:
            ...  # do stuff
        else:
            ...  # do other stuff

This is the test function I want to write

async def test_foo_timeout(httpx_mock, autojump_clock):
    async def cb(*args):
        await trio.sleep(some_long_time)
    
    httpx_mock.add_callback(cb)

    foo()

    ...  # assertions about what happens if the request times out

Importantly, autojump_clock virtualizes the clock so that my test executes immediately. It requires use of trio.sleep instead of other sleep functions, e.g. time.sleep.

Alternatively, I could inject a post function to foo and avoid mocking altogether, i.e.

async def foo(post):
    with trio.move_on_after(timeout) as cs:
        await post(endpoint, payload)
    if cs.cancelled_caught:
        ...  # do stuff
    else:
        ...  # do other stuff

In which case my test would become

async def test_foo_timeout(autojump_clock):
    async def post(*args):
        await trio.sleep(some_long_time)

    foo(post)

    ...  # assertions about what happens if the request times out

Let me know if this makes sense or there is another alternative which I didn't consider.

@Colin-b
Copy link
Owner

Colin-b commented Aug 9, 2021

Hello @calebho ,

Let me know if I understand your issue properly. You want your callback to trigger a timeout, not issued by httpx, but by trio so it can be catched by trio.move_on_after. Right?

And you cannot use time.sleep because it would not trigger the trio timeout. So you need an async call to trio.sleep instead.

Am I correct?

@calebho
Copy link
Contributor Author

calebho commented Aug 9, 2021

Yep that's right!

@Colin-b
Copy link
Owner

Colin-b commented Aug 9, 2021

Just to be sure it is actually something that can occur, this is not already handled in some way by httpx when you specify a timeout?

@calebho
Copy link
Contributor Author

calebho commented Aug 10, 2021

Not sure I understand fully, but do you mean rewriting my code to use the timeout parameter? I.e.

async def foo():
    async with httpx.AsyncClient() as c:
        try:
            await c.post(endpoint, json=payload, timeout=timeout)
        except httpx.TimeoutException:
            ... # do stuff
        else:
            ... # do other stuff

Test:

async def test_foo_timeout(httpx_mock, autojump_clock):
    def cb(*args):
        raise httpx.TimeoutException()
    
    httpx_mock.add_callback(cb)

    foo()

    ...  # assertions about what happens if the request times out

I think this is a workable solution for this simple case.

I think there are slightly more complex cases where something like this could be desirable. For instance suppose there was some other operation which you want to wrap in a timeout:

async def foo():
    async with httpx.AsyncClient() as c:
        with trio.move_on_after(timeout) as cs:
            async with trio.open_nursery() as n:
                n.start_soon(c.post, endpoint, json=payload)
                n.start_soon(some_other_long_running_op)
        if cs.cancelled_caught:
            ...  # do stuff
        else:
            ...  # do other stuff

It becomes a bit ugly to mix different timeout mechanisms (trio.move_on_after and timeout= parameter). Certainly more niche, but not super crazy.

Anyways, I think my original problem can be addressed without async callbacks, so feel free to close this out if adding this feature would take too much effort

@Colin-b
Copy link
Owner

Colin-b commented Aug 10, 2021

Indeed that was my question. If your concern can be addressed by httpx native timeout handling, then I would recommend using httpx timeout parameter.

I would like to keep code as clean as possible, and for that I would like to avoid adding unnecessary features. But if you are in the second use case, or if you ever encounter it, feel free to re-open this or another ticket.

Best Regards

@Colin-b
Copy link
Owner

Colin-b commented Oct 20, 2022

Hello @calebho, Release 0.21.1 is now available on pypi and allows you to register async callbacks.

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

No branches or pull requests

2 participants