-
Notifications
You must be signed in to change notification settings - Fork 33
fix: OpenFGA SDK doesn't seem to support async_req parameter #253
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughAdds support for passing the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
Codecov Report❌ Patch coverage is
❌ Your project status has failed because the head coverage (71.16%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #253 +/- ##
==========================================
- Coverage 71.17% 71.16% -0.01%
==========================================
Files 137 137
Lines 11100 11102 +2
==========================================
+ Hits 7900 7901 +1
- Misses 3200 3201 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@openfga_sdk/client/client.py`:
- Around line 123-124: Reject the async_req option in the async client path:
locate the code that checks options.get("async_req") and currently sets
kwargs["async_req"], and change it to raise a clear ValueError (or TypeError)
when options["async_req"] is not None, explaining that async_req is incompatible
with the async client (it triggers self.pool.apply_async and returns a
non-awaitable). Ensure the error message references async_req and the async
client so callers get an explicit, actionable failure instead of a silent
runtime error.
SoulPancake
left a 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.
I see that it initially was added to the async client too
Please update the PR description - Remove the line about async client changes since you've already removed that code
You checked "I have added tests" but there are currently no test files are in the diff
|
hi @SoulPancake , done |
Description
What problem is being solved?
The
async_reqparameter exists in the lower-levelapi_client.pyto enable threading-based asynchronous requests using Python'sThreadPool. However, this parameter is not exposed through the publicOpenFgaClientAPI because theoptions_to_kwargsfunction doesn't support it, making it inaccessible without using internal APIs.How is it being solved?
Updated the
options_to_kwargsfunction to pass through theasync_reqparameter from user options to the underlying API calls.What changes are made to solve it?
async_reqparameter support inopenfga_sdk/sync/client/client.py(sync client)After this change, users can use
async_reqlike this:References
closes #194
Review Checklist
mainSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.