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

Unclosed httpx.AsyncClient warning #6

Closed
jeanmonet opened this issue Feb 4, 2021 · 8 comments
Closed

Unclosed httpx.AsyncClient warning #6

jeanmonet opened this issue Feb 4, 2021 · 8 comments
Assignees

Comments

@jeanmonet
Copy link

Hi - thank you for this great (and light) library!

Upon exiting the script, I often get the following warning:

/lib/python3.9/site-packages/httpx/_client.py:1781: UserWarning: Unclosed <httpx.AsyncClient object at 0x7ac57ff51310>.
See https://www.python-httpx.org/async/#opening-and-closing-clients for details.

I'm not really familiar with the httpx library so not sure what this implies. Should the bitcoinrpc library provide a call to close the httpxAsyncClient?

@bibajz
Copy link
Owner

bibajz commented Feb 5, 2021

Hello, thank you for reporting the issue!

It was about time I looked on this library again, possibly adding some new features, since bitcoin core 0.21 was released...

The warning you receive was introduced here: encode/httpx#1197. I will take a closer look on it during this weekend and once again, thank you for reporting.

Libor

@bibajz bibajz self-assigned this Feb 5, 2021
@feavilan04
Copy link

Hi there.

Thanks for the library.

I'm using the library and getting the same error @jeanmonet quote above.

Please help us to check it up.

@bibajz
Copy link
Owner

bibajz commented Apr 14, 2021

Hey guys, sorry for the late response, I was partly busy IRL and partly slacking... :D

Since I am not near the Bitcoin node I operate and therefore unable to query it, I looked around the interwebs for running regtest node without minimum hussle and found a docker image.

It is not up to date, running Bitcoin Core v0.17.1 but it is sufficient for now - This commit 645c25d adds a simple script for running regtest locally on port 18443 with rpc_user/rpc_password credentials.

This also opens a possibility for better testing, since I can setup/teardown the regtest container in an instant - stay tuned! :)

But, to your problem, I have confirmed it and had the same issue with this simple script run from CLI:

import asyncio

from bitcoinrpc import BitcoinRPC


async def main():
    c = BitcoinRPC("localhost", 18443, "rpc_user", "rpc_password")
    print(await c.getbestblockhash())

if __name__ == "__main__":
    asyncio.run(main())

To close the httpx.AsyncClient, I have pushed 19aad69, which adds a BitcoinRPC.aclose coroutine which can be called to close the client.

Slightly modified script from above:

import asyncio

from bitcoinrpc import BitcoinRPC


async def main():
    c = BitcoinRPC("localhost", 18443, "rpc_user", "rpc_password")
    print(await c.getbestblockhash())
    await c.aclose()

if __name__ == "__main__":
    asyncio.run(main())

no longer emits the warning.

However, remembering to aclose the client everytime feels repetitive and also can be forgotten - I am thinking about adding the possibility to use BitcoinRPC as an asynchronous context manager, so the closing would be managed automatically, like this:

async with BitcoinRPC(...) as client:
    ...

What do you think @feavilan04, @jeanmonet ? Anyway, I will push the new version of this package to PyPI in few days.

Thank you both for your feedback and I promise my responses will be faster next time!

Libor

@feavilan04
Copy link

Thanks a lot @bibajz for the library, the last approach seems perfect for the job, let me know if you need anything.

Regards.

@jeanmonet
Copy link
Author

@bibajz

However, remembering to aclose the client everytime feels repetitive and also can be forgotten - I am thinking about adding the possibility to use BitcoinRPC as an asynchronous context manager, so the closing would be managed automatically, like this:

async with BitcoinRPC(...) as client:
    ...

The first solution using explicit .aclose() was indeed necessary. Considering this new requirement, your idea to make an async context manager makes a lot of sense IMO. If it doesn't make things complicated, dunno if both possibilities should be allowed (context manager + existing api)? Thanks for maintaining!

@bibajz
Copy link
Owner

bibajz commented Apr 15, 2021

Hey @jeanmonet, indeed it was... :)

I have implemented async context manager today, pushing it soon. Both options, calling the BitcoinRPC.aclose coroutine as well as using the async context manager, will be possible.

@bibajz
Copy link
Owner

bibajz commented Apr 15, 2021

OK, new version - v0.4.0 - is out! With aclose and async context manager - https://pypi.org/project/bitcoinrpc/0.4.0/

Thanks a lot! @jeanmonet @feavilan04

@jeanmonet
Copy link
Author

OK, new version - v0.4.0 - is out! With aclose and async context manager - https://pypi.org/project/bitcoinrpc/0.4.0/

Thanks a lot! @jeanmonet @feavilan04

Awesome thanks!

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

3 participants