chore: sync sdk generator changes #221
Conversation
WalkthroughUpdates the generator manifest, reorganizes docs and examples, adds/clarifies API method docstrings, and adjusts tests (formatting, one async→sync change, added client cleanup). No functional SDK code changes; primarily documentation and manifest exposure of additional model modules and example scaffolding. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Pre-merge checks (3 passed)✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
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✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #221 +/- ##
=======================================
Coverage 70.70% 70.70%
=======================================
Files 134 134
Lines 10881 10881
=======================================
Hits 7693 7693
Misses 3188 3188 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
openfga_sdk/sync/open_fga_api.py (1)
43-47: Context manager bug: exit signature is invalid (will raise TypeError).
__exit__must accept (exc_type, exc_value, traceback). Current signature breakswith OpenFgaApi(...) as ...:usage.- def __exit__(self): - self.close() + def __exit__(self, exc_type, exc_value, traceback): + self.close() + # returning None propagates exceptionstest/sync/oauth2_test.py (1)
343-393: Test name/docs vs behavior mismatch (uses 429s but says “5xx”).Either rename the test or include a 5xx in the sequence. Suggest exercising both retryable classes.
- def test_get_authentication_retries_5xx_responses(self, mock_request): + def test_get_authentication_retries_retryable_responses(self, mock_request): @@ - """ - Receiving a 5xx response from the server should be retried - """ + """ + Receiving retryable responses (429 and 5xx) should be retried. + """ @@ - mock_request.side_effect = [ - mock_response(error_response_body, 429), - mock_response(error_response_body, 429), - mock_response(error_response_body, 429), - mock_response(response_body, 200), - ] + mock_request.side_effect = [ + mock_response(error_response_body, 429), + mock_response(error_response_body, 503), + mock_response(error_response_body, 429), + mock_response(response_body, 200), + ]docs/OpenFgaApi.md (1)
1-1542: Fix broken Python examples (remove extra.api_instance) and correct 'conisistency' typo
- Replace all example calls like
api_instance.api_instance.<method>(...)withawait api_instance.<method>(...)in docs/OpenFgaApi.md (occurrences at lines: 70, 154, 236, 319, 401, 484, 567, 650, 736, 820, 904, 988, 1073, 1161, 1248, 1332, 1417, 1501).- Fix the typo
conisistency→consistencyin docs/OpenFgaApi.md (≈ lines 116, 529) and in generated SDK docstrings: openfga_sdk/sync/open_fga_api.py and openfga_sdk/api/open_fga_api.py (docstring occurrences around lines ~233, ~260, ~1067–1097).If docs/SDK files are generated, update the generator/templates to prevent recurrence.
openfga_sdk/api/open_fga_api.py (2)
100-106: Docs/code kwarg mismatch:_retry_param(docs) vs_retry_params(accepted). Accept both to avoid FgaValidationException.Right now, passing
_retry_param(as documented) raises “unexpected keyword argument.” Either update docs everywhere to_retry_paramsor accept both and normalize.all_params.extend( [ "async_req", "_return_http_data_only", "_preload_content", "_request_timeout", "_request_auth", "_content_type", "_headers", - "_retry_params", + "_retry_param", + "_retry_params", "_streaming", ] ) @@ - del local_var_params["kwargs"] + del local_var_params["kwargs"] + # Back-compat: normalize singular '_retry_param' to '_retry_params' + if local_var_params.get("_retry_param") is not None and local_var_params.get("_retry_params") is None: + local_var_params["_retry_params"] = local_var_params["_retry_param"]If you prefer doc-only changes, I can generate a follow-up patch to replace all
_retry_parammentions with_retry_paramsacross this file. Want that?Also applies to: 122-126
235-235: Fix user-facing typos and minor grammar in docstringsThese appear multiple times and are visible to users.
- “conisistency” → “consistency” (e.g., Lines 235, 1096)
- “viceversa” → “vice versa” (Line 235 block)
- “the the ListObjects API” → “the ListObjects API” (Lines 2489 and 2516 blocks)
- “along side” → “alongside” (Lines 2316, 2350)
- “an user” → “a user” (Line 2670 block)
I can submit a sweep commit if you want.
Also applies to: 1096-1096, 2489-2489, 2516-2516, 2316-2316, 2350-2350, 2670-2670
🧹 Nitpick comments (10)
README.md (1)
831-834: Clarify error semantics and result ordering for client_batch_check.
- Make explicit that
allowed: falsedue to an error is an indeterminate result and should be handled by inspectingerror.- Note that responses may not preserve input order; pairing should rely on the echoed
request.-If you are using an OpenFGA version less than 1.8.0, you can use `client_batch_check`, -which calls `check` in parallel. It will return `allowed: false` if it encounters an error, and will return the error in the body. -If 429s or 5xxs are encountered, the underlying check will retry up to 3 times before giving up. +If you are using an OpenFGA version less than 1.8.0, you can use `client_batch_check`, +which calls `check` in parallel. On error, each item will include an `error` and set `allowed: false` (treat as indeterminate, not a definitive denial). +Result ordering is not guaranteed; pair results using the echoed `request` field. +If 429s or 5xxs are encountered, the underlying `check` will retry up to 3 times before giving up (see "Retries" below).openfga_sdk/sync/open_fga_api.py (5)
52-52: Docstring: align type names and wording; keep NOTE concise.
- Prefer model names used in docs: “BatchCheckSingleResult” instead of "BatchCheckItem response".
- Keep “SDK can split requests” but avoid redundancy.
-returns a map containing `BatchCheckItem` response for each check it received. +returns a map of `correlation_id` → `BatchCheckSingleResult` for each check.
101-104: Docstring parameter name mismatch:_retry_paramvs_retry_params.The implementation uses
_retry_params; fix the docstring to match.- :param _retry_param: if specified, override the retry parameters specified in configuration + :param _retry_params: if specified, override the retry parameters specified in configuration
233-261: Typos in user-facing docstring.
- “conisistency” → “consistency”
- “viceversa” → “vice versa”
- higher conisistency + higher consistency - and viceversa. + and vice versa.
260-289: Docstring parameter name mismatch in check_with_http_info.Same
_retry_paramvs_retry_paramsinconsistency.- :param _retry_param: if specified, override the retry parameters specified in configuration + :param _retry_params: if specified, override the retry parameters specified in configuration
1067-1096: Typos in ListObjects docstrings.
- “conisistency” → “consistency”
- higher conisistency + higher consistencydocs/OpenFgaApi.md (2)
32-32: Tighten NOTE and fix lint (MD038: spaces inside code spans).
- Replace “BatchCheckItem” with “BatchCheckSingleResult”.
- Remove any spaces inside backticked code spans.
- Keep the NOTE concise.
- NOTE: The maximum number of checks that can be passed in the `BatchCheck` API is configurable via the [OPENFGA_MAX_CHECKS_PER_BATCH_CHECK](https://openfga.dev/docs/getting-started/setup-openfga/configuration#OPENFGA_MAX_CHECKS_PER_BATCH_CHECK) environment variable. If `BatchCheck` is called using the SDK, the SDK can split the batch check requests for you. + NOTE: The maximum number of checks in `BatchCheck` is configurable via [`OPENFGA_MAX_CHECKS_PER_BATCH_CHECK`](https://openfga.dev/docs/getting-started/setup-openfga/configuration#OPENFGA_MAX_CHECKS_PER_BATCH_CHECK). When called via the SDK, large batches may be auto-split.Also update:
-returns a map containing `BatchCheckItem` response +returns a map of `correlation_id` → `BatchCheckSingleResult`
782-783: Clarify bullet 3 and fix MD038.
- Clarify that
tuple_key.usermust be fully qualified whentuple_key.objectis type-only.- Ensure no spaces inside backticks.
-3. `tuple_key.user` is mandatory if tuple_key is specified in the case the `tuple_key.object` is a type only. If tuple_key.user is specified, it needs to be a full object (e.g., `type:user_id`). +3. `tuple_key.user` is mandatory when `tuple_key.object` is type-only (e.g., `document:`) and must be fully qualified (e.g., `user:anne`).openfga_sdk/api/open_fga_api.py (2)
54-54: Clarify BatchCheck splitting applies to the high-level Client, not this low-level API classAs written, this may mislead users into expecting OpenFgaApi to split batches automatically. Suggest narrowing the scope to the high-level client.
- NOTE: The maximum number of checks that can be passed in the `BatchCheck` API is configurable via the [OPENFGA_MAX_CHECKS_PER_BATCH_CHECK](https://openfga.dev/docs/getting-started/setup-openfga/configuration#OPENFGA_MAX_CHECKS_PER_BATCH_CHECK) environment variable. If `BatchCheck` is called using the SDK, the SDK can split the batch check requests for you. + NOTE: The maximum number of checks that can be passed in the `BatchCheck` API is configurable via the [OPENFGA_MAX_CHECKS_PER_BATCH_CHECK](https://openfga.dev/docs/getting-started/setup-openfga/configuration#OPENFGA_MAX_CHECKS_PER_BATCH_CHECK) environment variable. If you call `BatchCheck` via the high-level client (openfga_sdk.client.Client), the SDK may split batch check requests automatically. Direct calls via OpenFgaApi send the batch as-is.
81-81: Mirror clarification in with_http_info variantKeep the scope clarification consistent here as well.
- NOTE: The maximum number of checks that can be passed in the `BatchCheck` API is configurable via the [OPENFGA_MAX_CHECKS_PER_BATCH_CHECK](https://openfga.dev/docs/getting-started/setup-openfga/configuration#OPENFGA_MAX_CHECKS_PER_BATCH_CHECK) environment variable. If `BatchCheck` is called using the SDK, the SDK can split the batch check requests for you. + NOTE: The maximum number of checks that can be passed in the `BatchCheck` API is configurable via the [OPENFGA_MAX_CHECKS_PER_BATCH_CHECK](https://openfga.dev/docs/getting-started/setup-openfga/configuration#OPENFGA_MAX_CHECKS_PER_BATCH_CHECK) environment variable. If you call `BatchCheck` via the high-level client (openfga_sdk.client.Client), the SDK may split batch check requests automatically. Direct calls via OpenFgaApi send the batch as-is.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.openapi-generator/FILES(6 hunks)README.md(1 hunks)docs/OpenFgaApi.md(2 hunks)openfga_sdk/api/open_fga_api.py(4 hunks)openfga_sdk/sync/open_fga_api.py(4 hunks)test/oauth2_test.py(2 hunks)test/sync/oauth2_test.py(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
test/oauth2_test.py (1)
test/sync/oauth2_test.py (2)
test_get_authentication_obtain_client_credentials_with_scopes_list(108-164)test_get_authentication_obtain_client_credentials_with_scopes_string(167-223)
test/sync/oauth2_test.py (4)
openfga_sdk/sync/open_fga_api.py (1)
close(46-47)openfga_sdk/sync/api_client.py (1)
close(121-128)openfga_sdk/sync/client/client.py (1)
close(178-179)openfga_sdk/sync/rest.py (1)
close(218-222)
🪛 markdownlint-cli2 (0.17.2)
docs/OpenFgaApi.md
32-32: Spaces inside code span elements
(MD038, no-space-in-code)
32-32: Spaces inside code span elements
(MD038, no-space-in-code)
782-782: Spaces inside code span elements
(MD038, no-space-in-code)
782-782: Spaces inside code span elements
(MD038, no-space-in-code)
782-782: Spaces inside code span elements
(MD038, no-space-in-code)
782-782: Spaces inside code span elements
(MD038, no-space-in-code)
782-782: Spaces inside code span elements
(MD038, no-space-in-code)
782-782: Spaces inside code span elements
(MD038, no-space-in-code)
🔇 Additional comments (6)
test/sync/oauth2_test.py (2)
108-110: LGTM: multiline signature improves readability.No behavior change; keeping it.
166-169: LGTM: multiline signature + explicit resource cleanup.Adding
rest_client.close()avoids leaked connections.Also applies to: 223-223
openfga_sdk/api/open_fga_api.py (2)
1602-1602: LGTM: clearer Read input constraintsThe “tuple_key.user must be a full object when object is type-only” addition aligns with server behavior and prevents ambiguous inputs.
1629-1629: LGTM: same clarification for read_with_http_infoConsistent, accurate, and helpful.
test/oauth2_test.py (2)
499-501: LGTM: multiline signature improves readabilityNo behavioral change; consistent with surrounding style.
558-560: LGTM: same hereMatches formatting of other tests; no logic impact.
3 participants
Description
What problem is being solved?
How is it being solved?
What changes are made to solve it?
References
Review Checklist
mainSummary by CodeRabbit
New Features
Documentation
Tests
Chores