-
Notifications
You must be signed in to change notification settings - Fork 23
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
Add native async support for subscriptions #231
Comments
Curious about the scope of this. Is this just making the sub async compatible, or the worker as well? I havent delved into it, but do you know if the google cloud pubsub library supports async as well? |
It looks like the gcp pub/sub library doesn't support async yet - googleapis/python-pubsub#389 What are the tradeoffs of making the worker async-compatible vs just the sub? It's probably more complicated than this, but I was thinking you could just check if self.func was async and use asyncio.run(or similar) here. Line 90 in 5d05758
|
The subscriptions are essentially callbacks that are run within the worker. As such, a naive approach could be something you suggest with |
Basically
If you want to be able to run TONS of concurrent callbacks that do exclusively io, I think you'd need native support for async. For us, we're less concerned with the performance implications and just want to be able to inter-operate with async functions in our codebase. Understandable if this isn't a priority right now, but wanted to file an issue since it seems like this library is mostly about ergonomics. |
Not too sure if itll work, but one attempt you can try is sub-classing the Subscription class like here but with the asyncio.run in the call method.
|
Makes sense. I think I'll stick with the workaround for now. It's actually not so bad. Appreciate all the help ❤️ |
Currently, async functions can't be used with the @sub context manager.
If you try, you'll get the following error.
There are some simple workarounds, but it would be nice if this was supported natively.
The text was updated successfully, but these errors were encountered: