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

Feature request: Warn on blocking call allowed #34

Open
tomasfarias opened this issue Feb 1, 2025 · 3 comments
Open

Feature request: Warn on blocking call allowed #34

tomasfarias opened this issue Feb 1, 2025 · 3 comments

Comments

@tomasfarias
Copy link

I work with dependencies that implement asynchronous APIs but for one reason or another cannot remove all of their blocking calls1. Naturally, following blockbuster's README, we have a lot of these blocking calls allowed by calling can_block_in in our test suite.

However, I would like to be informed when:

  1. No blocking call happened even though a can_block_in rule was set.
  2. A blocking call happened, and it was allowed because of a can_block_in rule.

The first point could be useful to remove can_block_in calls as my dependencies slowly remove any remaining blocking calls.

The second point is more personal: I have a pytest.fixture that initializes a blockbuster_ctx and calls can_block_in a bunch of times to allow blocking calls from dependencies. The fixture is then passed around to all tests. I fear that me or my team will eventually forget which blocking calls we were allowing, and that this will cause us to underestimate how much we are blocking the event loop. Thus, I would like to be reminded of what I am allowing.

This could be achieved by raising a warning somewhere in the wrapper code, although I haven't looked closely enough. Happy to dig more and put up a PR if this is considered relevant!

Sidenote: Maybe can_block_in could allow all blocking calls from a particular path/module? This way only one warning would be raised.

Finally, thanks for your work in blockbuster! I had been looking for something like this for a while after many a production issue due to an unexpected blocking call going on for too long in code that seemed non-blocking.

Footnotes

  1. As an example, here is some discussion going on in aiobotocore due to blocking filesystem calls.

@cbornet
Copy link
Owner

cbornet commented Feb 3, 2025

Hi Tomas,
Thanks for your feedback, and happy you find blockbuster useful.
I think we could give the possibility to register a callback in can_block_in.
This way, a lot of behaviors can be implemented on the user side: increment counters, log, and so on.
For your use case you would implement:

  1. by incrementing a counter or setting a was_called boolean in the callback and checking it after the yield in your fixture.
  2. by warning in the callback, maybe with code to warn only once. One advantage of this is that you have access to the traceback that you could print in the warning.

One thing to be aware of is that some wrapped functions are really called a lot (eg: threading.Lock.acquire by the debugger). So the code in the callback should be very light.

Sidenote: Maybe can_block_in could allow all blocking calls from a particular path/module? This way only one warning would be raised.

Yes, I've been thinking about it too.
There could be a can_block_in at the BlockBuster class level and optionally pass a BlockBuster instance to the BlockBusterFunction, then to the _wrap_blocking.

WDYT ? Would that fit your need ?

@tomasfarias
Copy link
Author

I think we could give the possibility to register a callback in can_block_in.

This idea seems to offer a lot of flexibility, more than I would need, but it would definitely help with the warnings I want to raise.

There could be a can_block_in at the BlockBuster class level and optionally pass a BlockBuster instance to the BlockBusterFunction, then to the _wrap_blocking.

The current branch I'm working with has a test file that it's pretty much allowing every os function in different aioboto3 modules as it does blocking calls with os a lot while initializing. So, this would help a ton in streamline some of the code.

Happy to work on a PR for these two features!

@cbornet
Copy link
Owner

cbornet commented Feb 19, 2025

The current branch I'm working with has a test file that it's pretty much allowing every os function in different aioboto3 modules as it does blocking calls with os a lot while initializing. So, this would help a ton in streamline some of the code.

Note that you can already do something like this: https://github.com/langchain-ai/langchain/blob/6c1e21d1282a824fea52225f7b87e24a4edc95d7/libs/core/tests/unit_tests/conftest.py#L31C9-L34C14
to allow all functions to block.

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

2 participants