Skip to content

Fix #4 - Handle thread safety for shared clients (Celery fixes)#17

Open
Deadpool2000 wants to merge 9 commits intoopenapi:mainfrom
Deadpool2000:fix-4
Open

Fix #4 - Handle thread safety for shared clients (Celery fixes)#17
Deadpool2000 wants to merge 9 commits intoopenapi:mainfrom
Deadpool2000:fix-4

Conversation

@Deadpool2000
Copy link
Copy Markdown
Contributor

@Deadpool2000 Deadpool2000 commented Apr 9, 2026

hey @francescobianco ,

this is fix for #4

I’ve been looking into the issue where OauthClient was blowing up when shared across Celery workers, and it’s basically down to how we were handling the internal httpx client. Since we were initializing it once in the constructor, every single worker thread was trying to squeeze through the same connection pool at the same time. That’s why users were seeing those random failures and corrupted data - the internal state just couldn't handle that much concurrent noise.

To fix this without making everyone rewrite their Celery tasks, I moved the internal client over to a property backed by threading.local(). So now, even if you define the client once at the top of your file, each worker thread will lazily spin up its own private connection the first time it actually needs to make a call. It completely solves the thread-safety headache while keeping the code looking exactly the same from the outside.

One thing I wanted to be careful about was not breaking the custom client injection. I know some people pass in their own sessions for custom retries or proxies, so I made sure that if you explicitly pass a client in the constructor, it still uses that exact object across all threads. This keeps things backward compatible for power users while making the default behavior safe for everyone else.

I also threw in a new test file, tests/test_thread_safety.py, which just fires off a bunch of threads and makes sure they aren't stepping on each other's toes by checking that they each get a unique client instance. All the original tests pass too, since the mocks still hook into the property correctly.

Let me know what you think!

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

Successfully merging this pull request may close these issues.

1 participant