chore: reduce generated code #236
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project status has failed because the head coverage (70.86%) 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 #236 +/- ##
==========================================
+ Coverage 70.79% 70.86% +0.07%
==========================================
Files 134 135 +1
Lines 10904 10932 +28
==========================================
+ Hits 7719 7747 +28
Misses 3185 3185 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Important Review skippedAuto incremental 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 Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThis PR centralizes magic constants scattered across the OpenFGA Python SDK codebase into a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes The PR is mostly consistent patterns applied across many files: centralized constants (straightforward), refactored hard-coded values to use those constants (repetitive), and numerous docstring removals (trivial but numerous). The logic changes are isolated to configuration and client files, but the sheer file count (~80+) warrants moderate review scope despite repetitive patterns. Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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.
Pull Request Overview
This PR reduces generated code by consolidating constants into a centralized file and removing redundant auto-generated headers from test and SDK files. The changes improve maintainability by eliminating code duplication and establishing a single source of truth for SDK-wide constants.
- Removed auto-generated file headers from 60+ test and source files
- Created centralized
openfga_sdk/constants.pywith all SDK constants marked asFinal - Refactored hardcoded values to reference centralized constants across client, configuration, and API modules
Reviewed Changes
Copilot reviewed 70 out of 70 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| openfga_sdk/constants.py | New centralized constants file with SDK version, URLs, retry configs, client settings, and header names |
| openfga_sdk/init.py | Updated to import SDK_VERSION from constants module |
| openfga_sdk/configuration.py | Refactored to use constants from centralized file for retry parameters and version reporting |
| openfga_sdk/api_client.py | Updated to import and use constants for user agent, timeouts, and retry parameters |
| openfga_sdk/sync/api_client.py | Parallel changes to async api_client for constants usage |
| openfga_sdk/client/client.py | Updated to use constants for client method headers and batch settings |
| openfga_sdk/sync/client/client.py | Parallel changes to async client for constants usage |
| openfga_sdk/oauth2.py | Updated to use USER_AGENT constant |
| openfga_sdk/sync/oauth2.py | Parallel changes to async oauth2 for USER_AGENT constant |
| test/constants_consistency_test.py | New test file ensuring consistency between constants and their usage across modules |
| test/*/*_test.py | Removed auto-generated headers from all test files |
| README.md | Simplified contributing section to reference CONTRIBUTING.md |
| CONTRIBUTING.md | Enhanced with clearer guidance on autogenerated files and PR submission process |
| .openapi-generator/FILES | Updated to reflect new constants file and removed obsolete entries |
| .openapi-generator-ignore | Updated to exclude more autogenerated files from regeneration |
Comments suppressed due to low confidence (1)
openfga_sdk/constants.py:1
- Corrected spelling of 'recieve' to 'receive' in comment on line 5 of constants.py
"""
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
openfga_sdk/api_client.py (1)
251-259: Bug: _retry_params.min_wait_in_ms assigned to wrong variable in two files.The minimum wait time parameter is being assigned to
max_retryinstead ofmin_wait_in_ms, breaking backoff timing and causing type confusion.Fix required in:
openfga_sdk/api_client.pyline 257openfga_sdk/sync/api_client.pyline 256if _retry_params is not None: if _retry_params.max_retry is not None: max_retry = _retry_params.max_retry if _retry_params.min_wait_in_ms is not None: - max_retry = _retry_params.min_wait_in_ms + min_wait_in_ms = _retry_params.min_wait_in_ms if _retry_params.max_wait_in_sec is not None: max_wait_in_sec = _retry_params.max_wait_in_sec
🧹 Nitpick comments (2)
test/oauth2_test.py (1)
1720-1759: Make the final 200 OK payload match CheckResponse to avoid false positives.The success body in this retry test still contains an error schema. Use a minimal valid CheckResponse to assert correctness end‑to‑end.
- response_body = """ -{ - "code": "internal_error", - "message": "Internal Server Error" -} - """ + # Use a valid CheckResponse shape for the final 200 OK + response_body = """ +{ + "allowed": true +} + """openfga_sdk/constants.py (1)
17-20: Consider constructing USER_AGENT from SDK_VERSION to eliminate duplication.The version "0.9.7" appears in both
SDK_VERSIONandUSER_AGENT, creating a potential desynchronization risk. While the test attest/constants_consistency_test.py:47-50validates thatSDK_VERSIONis present inUSER_AGENT, constructing the user agent string dynamically would be more robust:SDK_VERSION: Final[str] = "0.9.7" USER_AGENT: Final[str] = f"openfga-sdk python/{SDK_VERSION}"Note: Given that line 10 indicates this file is auto-generated, this change may need to be made in the generation template rather than directly in this file.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (70)
.openapi-generator-ignore(1 hunks).openapi-generator/FILES(1 hunks)CONTRIBUTING.md(1 hunks)README.md(2 hunks)openfga_sdk/__init__.py(2 hunks)openfga_sdk/api_client.py(3 hunks)openfga_sdk/client/__init__.py(0 hunks)openfga_sdk/client/client.py(4 hunks)openfga_sdk/client/configuration.py(0 hunks)openfga_sdk/client/models/__init__.py(0 hunks)openfga_sdk/client/models/assertion.py(0 hunks)openfga_sdk/client/models/batch_check_item.py(0 hunks)openfga_sdk/client/models/batch_check_request.py(0 hunks)openfga_sdk/client/models/batch_check_response.py(0 hunks)openfga_sdk/client/models/batch_check_single_response.py(0 hunks)openfga_sdk/client/models/check_request.py(0 hunks)openfga_sdk/client/models/client_batch_check_response.py(0 hunks)openfga_sdk/client/models/expand_request.py(0 hunks)openfga_sdk/client/models/list_objects_request.py(0 hunks)openfga_sdk/client/models/list_relations_request.py(0 hunks)openfga_sdk/client/models/list_users_request.py(0 hunks)openfga_sdk/client/models/read_changes_request.py(0 hunks)openfga_sdk/client/models/tuple.py(0 hunks)openfga_sdk/client/models/write_request.py(0 hunks)openfga_sdk/client/models/write_response.py(0 hunks)openfga_sdk/client/models/write_single_response.py(0 hunks)openfga_sdk/client/models/write_transaction_opts.py(0 hunks)openfga_sdk/configuration.py(5 hunks)openfga_sdk/constants.py(1 hunks)openfga_sdk/credentials.py(0 hunks)openfga_sdk/help.py(0 hunks)openfga_sdk/oauth2.py(2 hunks)openfga_sdk/rest.py(0 hunks)openfga_sdk/sync/__init__.py(0 hunks)openfga_sdk/sync/api_client.py(3 hunks)openfga_sdk/sync/client/__init__.py(0 hunks)openfga_sdk/sync/client/client.py(4 hunks)openfga_sdk/sync/oauth2.py(2 hunks)openfga_sdk/sync/rest.py(0 hunks)openfga_sdk/telemetry/__init__.py(0 hunks)openfga_sdk/telemetry/attributes.py(0 hunks)openfga_sdk/telemetry/configuration.py(0 hunks)openfga_sdk/telemetry/counters.py(0 hunks)openfga_sdk/telemetry/histograms.py(0 hunks)openfga_sdk/telemetry/metrics.py(0 hunks)openfga_sdk/telemetry/telemetry.py(0 hunks)openfga_sdk/validation.py(0 hunks)test/__init__.py(0 hunks)test/api/__init__.py(0 hunks)test/api/open_fga_api_test.py(5 hunks)test/client/__init__.py(0 hunks)test/client/client_test.py(0 hunks)test/configuration_test.py(0 hunks)test/constants_consistency_test.py(1 hunks)test/credentials_test.py(0 hunks)test/oauth2_test.py(8 hunks)test/rest_test.py(0 hunks)test/sync/__init__.py(0 hunks)test/sync/client/__init__.py(0 hunks)test/sync/client/client_test.py(0 hunks)test/sync/oauth2_test.py(4 hunks)test/sync/open_fga_api_test.py(6 hunks)test/sync/rest_test.py(0 hunks)test/telemetry/attributes_test.py(0 hunks)test/telemetry/configuration_test.py(0 hunks)test/telemetry/counters_test.py(0 hunks)test/telemetry/histograms_test.py(0 hunks)test/telemetry/metrics_test.py(0 hunks)test/telemetry/telemetry_test.py(0 hunks)test/validation_test.py(0 hunks)
💤 Files with no reviewable changes (52)
- openfga_sdk/sync/client/init.py
- test/telemetry/attributes_test.py
- openfga_sdk/client/models/read_changes_request.py
- openfga_sdk/client/models/init.py
- test/telemetry/histograms_test.py
- test/init.py
- test/sync/rest_test.py
- test/sync/client/init.py
- test/credentials_test.py
- openfga_sdk/telemetry/counters.py
- openfga_sdk/client/models/list_relations_request.py
- openfga_sdk/client/models/tuple.py
- test/sync/client/client_test.py
- openfga_sdk/telemetry/histograms.py
- openfga_sdk/client/models/assertion.py
- openfga_sdk/client/models/write_response.py
- openfga_sdk/client/models/list_objects_request.py
- openfga_sdk/telemetry/telemetry.py
- openfga_sdk/client/models/write_transaction_opts.py
- openfga_sdk/client/models/list_users_request.py
- openfga_sdk/client/models/batch_check_single_response.py
- openfga_sdk/sync/rest.py
- openfga_sdk/client/models/check_request.py
- openfga_sdk/telemetry/init.py
- test/configuration_test.py
- openfga_sdk/client/models/batch_check_response.py
- openfga_sdk/credentials.py
- openfga_sdk/client/models/batch_check_request.py
- test/telemetry/telemetry_test.py
- openfga_sdk/client/configuration.py
- openfga_sdk/client/init.py
- test/telemetry/configuration_test.py
- openfga_sdk/client/models/client_batch_check_response.py
- openfga_sdk/client/models/expand_request.py
- test/validation_test.py
- test/client/client_test.py
- openfga_sdk/rest.py
- openfga_sdk/telemetry/metrics.py
- openfga_sdk/client/models/batch_check_item.py
- openfga_sdk/sync/init.py
- openfga_sdk/help.py
- test/sync/init.py
- openfga_sdk/telemetry/attributes.py
- openfga_sdk/client/models/write_request.py
- test/rest_test.py
- openfga_sdk/client/models/write_single_response.py
- openfga_sdk/telemetry/configuration.py
- test/client/init.py
- test/api/init.py
- test/telemetry/metrics_test.py
- test/telemetry/counters_test.py
- openfga_sdk/validation.py
🧰 Additional context used
🧬 Code graph analysis (5)
openfga_sdk/sync/client/client.py (1)
openfga_sdk/client/models/write_transaction_opts.py (2)
max_parallel_requests(51-55)max_parallel_requests(58-65)
test/constants_consistency_test.py (1)
openfga_sdk/configuration.py (10)
api_url(701-703)api_url(706-708)retry_params(733-737)retry_params(740-744)max_retry(53-62)max_retry(65-79)min_wait_in_ms(82-86)min_wait_in_ms(89-98)max_wait_in_sec(101-105)max_wait_in_sec(108-117)
test/sync/open_fga_api_test.py (2)
openfga_sdk/api_client.py (1)
ApiClient(44-920)openfga_sdk/sync/api_client.py (1)
ApiClient(43-918)
openfga_sdk/configuration.py (1)
openfga_sdk/exceptions.py (1)
FgaValidationException(36-61)
openfga_sdk/client/client.py (1)
openfga_sdk/client/models/write_transaction_opts.py (2)
max_parallel_requests(51-55)max_parallel_requests(58-65)
🪛 LanguageTool
CONTRIBUTING.md
[style] ~39-~39: Consider using a less common alternative to make your writing sound more unique and professional.
Context: ...acker.** ### Submitting Pull Requests Feel free to submit a Pull Request against this repo...
(FEEL_FREE_TO_STYLE_ME)
🔇 Additional comments (19)
test/sync/oauth2_test.py (1)
8-8: Good switch to centralized USER_AGENT.Using openfga_sdk.constants.USER_AGENT keeps tests aligned with client defaults and reduces drift.
Also applies to: 76-77, 134-135, 193-194
openfga_sdk/oauth2.py (1)
12-12: Consistent User-Agent source.Importing USER_AGENT and using it in token requests is correct and removes hardcoded strings.
Also applies to: 82-83
test/oauth2_test.py (1)
9-9: LGTM on USER_AGENT centralization.All header expectations now use the shared constant; consistent with client defaults.
Also applies to: 76-77, 302-303, 358-359, 413-414, 526-527, 585-586
test/sync/open_fga_api_test.py (3)
12-12: Importing USER_AGENT here is correct.Keeps sync tests aligned with the new constants module.
1720-1759: Retry behavior test reads well.Covers multiple 5xx codes and a final success; call_count assertion matches side effects.
If you want parity with the async test, consider asserting a field on the parsed response as well.
1835-1836: Headers now use USER_AGENT constant — nice.Reduces duplication and future‑proofs tests against UA changes.
Also applies to: 1890-1891, 1947-1948, 2003-2004
openfga_sdk/__init__.py (1)
6-6: Version derives from SDK_VERSION.Good centralization; avoids manual bumps.
Also applies to: 134-135
openfga_sdk/api_client.py (2)
19-24: Centralized constants import is correct.Alias for DEFAULT_USER_AGENT preserves the public symbol while sourcing from constants.
418-421: Good guardrail for Retry‑After header.Using RETRY_HEADER_MAX_ALLOWABLE_DURATION_IN_SEC avoids unbounded sleeps.
openfga_sdk/configuration.py (5)
9-15: Correct consolidation of defaults and version.Imports bring retry/backoff and version constants into one place.
42-47: RetryParams defaults now come from constants.Appropriate centralization; matches the rest of the refactor.
57-60: Bound check on max_retry in getter.Prevents invalid configs from silently passing.
Consider mirroring a similar upper bound on max_wait_in_sec if you want to cap sleep even when not using Retry‑After.
74-77: Setter validation message is clear and constant‑driven.Error text references RETRY_MAX_ALLOWED_NUMBER; good.
546-547: Debug report reflects SDK_VERSION.Avoids stale version strings.
test/constants_consistency_test.py (1)
26-50: LGTM! Well-structured consistency tests.The test functions effectively validate alignment between constants and their usage across the SDK. The coverage includes version consistency, retry parameter defaults, and user agent constants for both sync and async clients.
openfga_sdk/sync/api_client.py (3)
17-21: LGTM! Constants properly centralized.The imports from
openfga_sdk.constantssuccessfully replace local definitions, improving maintainability. The aliasUSER_AGENT as DEFAULT_USER_AGENTpreserves the existing API surface.
244-251: LGTM! Hardcoded value replaced with constant.Replacing the hardcoded
120withMAX_BACKOFF_TIME_IN_SECimproves maintainability and ensures consistency across the codebase.
416-422: LGTM! Retry header validation uses centralized constant.Replacing the hardcoded
1800withRETRY_HEADER_MAX_ALLOWABLE_DURATION_IN_SECensures consistent behavior across retry logic and improves code clarity.openfga_sdk/constants.py (1)
1-93: LGTM! Well-organized constants module.The constants are logically grouped, comprehensively documented, and properly typed with
Final. This centralization will significantly improve maintainability across the SDK.
Co-authored-by: Copilot <[email protected]>
Description
Generated from: openfga/sdk-generator#638 using
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
Chores