-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Fix : Restore Async Usage in Outlook Mail Toolkit #3623
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
Conversation
Reverts changes from enhancement pr
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @Tanuj-Taneja1, thanks for this PR and Merry Christmas! 🎄
The reason we previously changed them to sync was to maintain API consistency for users - we wanted all toolkit methods to have a unified calling convention.
However, since msgraph-sdk doesn't support synchronous calls, I created one PR to update FunctionTool.__call__() in your branch: #3627
Now when async tools are called synchronously (e.g., via agent.step()), it will:
- Execute them correctly using
asyncio.run()(or in a thread pool if already in an async context) - Emit a RuntimeWarning suggesting users to use
await tool.async_call()orawait agent.astep()for better performance
This way we get the best of both worlds - async toolkits like this one will work correctly, and users who call them synchronously still get working code with a gentle nudge toward the async API
Yes that makes perfect sense, I mentioned a issue in #3627 if you could reproduce the issue do let me know. |
Description
It restores the required asynchronous behaviour in the Outlook Mail Toolkit by reintroducing async/await and updating the tests to support async execution. This is necessary because the msgraph-sdk does not support synchronous methods.
Also Merry Christmas Everyone
Checklist
Go over all the following points, and put an
xin all the boxes that apply.Fixes #issue-numberin the PR description (required)pyproject.tomlanduv lockIf you are unsure about any of these, don't hesitate to ask. We are here to help!