-
Notifications
You must be signed in to change notification settings - Fork 33
feat!: add support for BatchCheck API #154
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
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #154 +/- ##
==========================================
+ Coverage 68.95% 69.49% +0.53%
==========================================
Files 124 133 +9
Lines 9864 10554 +690
==========================================
+ Hits 6802 7334 +532
- Misses 3062 3220 +158 ☔ View full report in Codecov by Sentry. |
jimmyjames
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.
Generally looks great, a couple questions and a nit. I'll spend some more time in greater depth tomorrow looking at it.
|
Changes LGTM 👍 |
jimmyjames
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.
@ewanharris we should also update the repo's telemetry docs with the new attribute: https://github.com/openfga/python-sdk/blob/main/docs/opentelemetry.md
3458e44 to
911c3f9
Compare
BREAKING CHANGE: Usage of the existing batch_check should now use client_batch_check instead, additionally the existing BatchCheckResponse has been renamed to ClientBatchCheckClientResponse.
911c3f9 to
39cd0b0
Compare
39cd0b0 to
9e8399a
Compare
Helps show the correlation_id use
9e8399a to
36f5016
Compare
BREAKING CHANGE: Usage of the existing batch_check should now use client_batch_check instead,
additionally the existing BatchCheckResponse has been renamed to ClientBatchCheckClientResponse.
Description
This PR introduces a method for using the
BatchCheckAPI released in OpenFGA v1.8.0, in order to facilitate this it includes a breaking change where the existingbatch_checkmethod (and associated type) has been renamed toclient_batch_check. This is so that our naming conventions remain consistent.Migration
"I want to continue using client side batch check"
Given that this necessitates an upgrade of OpenFGA, it might not be feasible for everyone to switch to the new server based method, if you wish to delay the migration, rename the existing usage to the new method name as shown below.
"I want to migrate to the new server based batch check"
If you wish to migrate to the new method, whilst the method name remains the same. You will need to alter the way you construct the checks passed.
ClientCheckRequestwas constructed and passed directly tobatch_check, now you should construct a list ofClientBatchCheckItemand pass aClientBatchCheckRequesttobatch_checkwith that list as thecheckspropertycorrelation_idon aClientBatchCheckItemis set for you if you do not provide it.resultnow contains acorrelation_idproperty in addition to theerrorandrequesttypes and has removed theresponseproperty. Additionally, theerrorproperty is now of aCheckErrortype, rather than anExceptiontype.References
openfga/sdk-generator#459
Review Checklist
mainBREAKING CHANGE: Usage of the existing batch_check should now use client_batch_check instead,
additionally the existing BatchCheckResponse has been renamed to ClientBatchCheckClientResponse.