feat: add raw requestor for calling arbitrary endpoints #252
Conversation
WalkthroughThis PR introduces a new Changes
Sequence Diagram(s)sequenceDiagram
participant Client as OpenFgaClient
participant ApiClient as ApiClient
participant HTTP as HTTP Layer
participant Response as Response Parser
Client->>Client: raw_request(method, path, params...)
Client->>Client: Validate operation_name (required)
Client->>Client: Resolve path params (auto-fill store_id)
Client->>Client: URL-encode path params
Client->>Client: Build query string
Client->>Client: Serialize body (JSON if dict/list)
Client->>Client: Set default headers (Content-Type, Accept)
Client->>ApiClient: call_api(method, path, body, headers, query_params)
ApiClient->>HTTP: Execute HTTP request
HTTP->>ApiClient: Return HTTP response
ApiClient->>Response: Parse response body
Response->>Response: Try JSON decode, fallback to raw
Response->>Client: Return RawResponse(status, headers, body)
Client->>Client: Return RawResponse to caller
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
|
Related Documentation Checked 6 published document(s) in 1 knowledge base(s). No updates required. How did I do? Any feedback? |
SoulPancake
changed the title
kcbiradar
force-pushed
the
feat/add-raw-request-method
branch
from
840bc79 to
f4dec14
Compare
![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)
README.md (1)
4-4: Malformed badge URL - missing closing parenthesis.The Socket badge URL appears to be malformed. It's missing a closing parenthesis before the opening bracket for the link URL.
🔧 Suggested fix
-[ +[](https://socket.dev/pypi/package/openfga-sdk)
🤖 Fix all issues with AI agents
In `@openfga_sdk/sync/client/client.py`:
- Around line 1233-1234: The except block "except ApiException as e: raise" is
redundant and should be removed; locate the handler in
openfga_sdk/sync/client/client.py where ApiException is caught and simply delete
that except clause so the original ApiException will propagate naturally (leave
surrounding try/except structure and other exception handlers unchanged and
ensure indentation remains valid).
🧹 Nitpick comments (6)
openfga_sdk/client/models/raw_response.py (1)
49-63: Consider movingjsonimport to module level.The
jsonmodule is imported inside the method body (lines 50 and 58). Sincejsonis part of Python's standard library and always available, importing it at the module level (line 8-9 area) would be more idiomatic and slightly more efficient for repeated calls.♻️ Suggested refactor
Add import at the top of the file:
from dataclasses import dataclass from typing import Any +import jsonThen remove the inline imports in
json()andtext()methods.openfga_sdk/sync/client/client.py (2)
1139-1141: Consider validatingoptions["headers"]type before merging.The code assumes
options["headers"]is a dict when callingupdate(). For consistency with theset_heading_if_not_sethelper (lines 102-103) which handles non-dict headers values, consider adding a type check.♻️ Suggested defensive check
request_headers = dict(headers) if headers else {} if options and options.get("headers"): - request_headers.update(options["headers"]) + opt_headers = options.get("headers") + if isinstance(opt_headers, dict): + request_headers.update(opt_headers)
1196-1199: Header checks are case-sensitive but HTTP headers are case-insensitive.The checks
"Content-Type" not in request_headerswon't match if a user provides"content-type"(lowercase). This could result in duplicate headers with different casing.♻️ Suggested case-insensitive check
+ # Case-insensitive header check helper + request_headers_lower = {k.lower(): k for k in request_headers} + - if "Content-Type" not in request_headers: + if "content-type" not in request_headers_lower: request_headers["Content-Type"] = "application/json" - if "Accept" not in request_headers: + if "accept" not in request_headers_lower: request_headers["Accept"] = "application/json"test/sync/client/client_test.py (1)
4151-4454: Good raw_request coverage; consider 2 small edge-case tests.
- Assert that custom headers passed to
raw_request(..., headers=...)are actually forwarded (e.g., verifyX-Experimental-Featureis present in the mocked call headers).- Add a non-JSON response body case (e.g.,
content-type: text/plain) to validateRawResponse.text()/json()behavior when JSON parsing should fail.openfga_sdk/client/client.py (2)
1140-1143: Define header precedence + coerce header values to strings.Right now
options["headers"]overrides the explicitheadersargument (Line 1140-1143), and non-string values can slip in (your tests cover integer header values elsewhere). It’d be good to (a) document precedence, and (b) normalize values tostrbefore sending.
1209-1234: Tighten telemetry + response typing.
TelemetryAttributeimport inside the method is unused (Line 1211).TelemetryAttributes.fga_client_request_methodis set tooperation_name.lower()(Line 1213-1214). Please confirm this attribute is meant to hold the operation name vs the HTTP verb (since you already havemethod).- Response parsing should allow top-level JSON arrays too (currently annotated as
dict[str, Any], butjson.loads(...)can returnlist[Any]). Consider widening the type and aligningRawResponse.bodyaccordingly.Also applies to: 1242-1265
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
README.mdopenfga_sdk/api_client.pyopenfga_sdk/client/client.pyopenfga_sdk/client/models/__init__.pyopenfga_sdk/client/models/raw_response.pyopenfga_sdk/sync/api_client.pyopenfga_sdk/sync/client/client.pyopenfga_sdk/telemetry/attributes.pytest/client/client_test.pytest/sync/client/client_test.py
🧰 Additional context used
🧬 Code graph analysis (6)
openfga_sdk/client/models/__init__.py (1)
openfga_sdk/client/models/raw_response.py (1)
RawResponse(13-90)
openfga_sdk/sync/api_client.py (3)
openfga_sdk/rest.py (1)
getheaders(111-115)openfga_sdk/sync/rest.py (1)
getheaders(111-115)test/error_handling_test.py (1)
getheaders(31-32)
openfga_sdk/api_client.py (3)
openfga_sdk/rest.py (1)
getheaders(111-115)openfga_sdk/sync/rest.py (1)
getheaders(111-115)test/error_handling_test.py (1)
getheaders(31-32)
test/client/client_test.py (1)
openfga_sdk/client/models/raw_response.py (3)
RawResponse(13-90)json(33-65)text(67-90)
test/sync/client/client_test.py (2)
openfga_sdk/client/models/raw_response.py (3)
RawResponse(13-90)json(33-65)text(67-90)openfga_sdk/rest.py (6)
RESTClientObject(128-461)response(56-60)response(63-67)status(84-88)status(91-95)close(162-166)
openfga_sdk/client/client.py (6)
openfga_sdk/client/models/raw_response.py (2)
json(33-65)RawResponse(13-90)openfga_sdk/exceptions.py (2)
ApiException(118-298)FgaValidationException(36-61)openfga_sdk/sync/rest.py (5)
RESTResponse(27-125)data(70-74)data(77-81)status(84-88)status(91-95)openfga_sdk/telemetry/attributes.py (3)
get(97-104)TelemetryAttribute(14-16)TelemetryAttributes(19-369)openfga_sdk/api_client.py (2)
get_store_id(937-941)update_params_for_auth(771-810)openfga_sdk/sync/api_client.py (2)
get_store_id(937-941)update_params_for_auth(771-810)
🔇 Additional comments (17)
openfga_sdk/client/models/__init__.py (1)
26-26: LGTM!The
RawResponseimport and export follow the established pattern in this module. This correctly exposes the new model for downstream usage.Also applies to: 49-49
openfga_sdk/api_client.py (1)
420-423: LGTM!The change from
response_data.headerstoresponse_data.getheaders()ensures consistent header access and returns a properdict[str, str]that aligns with the newRawResponsemodel's expected header format.openfga_sdk/sync/api_client.py (1)
420-423: LGTM!Consistent with the async client change, ensuring both sync and async variants use the same
getheaders()method for header access.openfga_sdk/telemetry/attributes.py (1)
298-302: Good defensive fix for handling non-dict response bodies.The added
isinstance(response.body, dict)check properly guards againstAttributeErrorwhenbodyisbytes,str, orNone- which is possible with the newRawResponsemodel where body type isbytes | str | dict[str, Any] | None.README.md (2)
51-51: LGTM!Good addition to the table of contents for the new feature section.
1264-1400: Comprehensive documentation for the newraw_requestfeature.The documentation is well-structured with clear use cases and multiple examples covering:
- Basic POST requests
- Response handling (JSON parsing, accessing headers/status)
- GET requests with query parameters
- Path parameter substitution (explicit and automatic store_id)
The examples follow the existing documentation patterns in this README.
openfga_sdk/client/models/raw_response.py (1)
33-90: LGTM! The helper methods are well-implemented.The
json()andtext()methods handle all expected body types (dict, str, bytes, None) with appropriate error handling and fallbacks. The UnicodeDecodeError handling witherrors="replace"intext()is a good defensive pattern.openfga_sdk/sync/client/client.py (3)
1-4: LGTM! Imports are appropriate for the new functionality.The new imports (
json,urllib.parse,RawResponse,ApiException,RESTResponse) are all necessary for theraw_requestmethod implementation.Also applies to: 38-38, 46-46, 77-77
1146-1167: LGTM! Path parameter handling is well-implemented.The logic correctly:
- Auto-fills
store_idfrom configuration when not provided inpath_params- URL-encodes path parameter values for safety
- Validates that all placeholders are resolved before making the request
1243-1265: LGTM! Response body parsing handles all expected formats.The parsing logic correctly handles JSON responses (both string and bytes) with appropriate fallbacks for non-JSON content. The final
RawResponseconstruction properly captures status, headers, and body.test/client/client_test.py (5)
28-28: LGTM! Import is correctly placed.The
RawResponseimport is appropriately added alongside other model imports.
4170-4218: LGTM! Comprehensive test for POST request with body.This test verifies:
- RawResponse creation and properties (status, headers, body)
json()andtext()helper methods work correctly- The API is called with correct method, path, body, query params, and headers
4330-4379: LGTM! Validation error tests cover required scenarios.The tests properly verify that
FgaValidationExceptionis raised for:
- Missing
operation_name(required parameter)- Missing
store_idwhen path contains{store_id}- Missing path parameters when placeholders exist
4381-4413: LGTM! List query parameter expansion test.This test verifies that list values in
query_paramsare correctly expanded into multiple key-value pairs (e.g.,{"ids": ["id1", "id2", "id3"]}becomes[("ids", "id1"), ("ids", "id2"), ("ids", "id3")]).
4444-4475: LGTM! URL encoding test verifies special character handling.The test confirms that path parameters containing spaces and special characters (
value with spaces & special chars) are properly URL-encoded tovalue%20with%20spaces%20%26%20special%20chars.test/sync/client/client_test.py (1)
26-26: RawResponse import looks fine.openfga_sdk/client/client.py (1)
3-6: No issues with the new imports.Also applies to: 39-40, 46-51, 76-76
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
|
@rhamzeh @SoulPancake, please review and suggest if any changes are required. |
There was a problem hiding this comment.
Pull request overview
This PR adds a raw_request method to the OpenFGA Python SDK, enabling developers to call arbitrary FGA endpoints that may not yet be wrapped by the SDK. This is useful for calling experimental features, custom endpoints, or new API features in older SDK versions.
Changes:
- Added
RawResponsedataclass with status, headers, body, and helper methods (json(),text()) - Implemented
raw_request()method in both async and sync clients with path parameter substitution, query parameter handling, and automatic store_id resolution - Fixed API client header handling to use
getheaders()for proper dict conversion
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| openfga_sdk/client/models/raw_response.py | New RawResponse model with status, headers, body, and utility methods |
| openfga_sdk/client/client.py | Async raw_request implementation with URL encoding, query params, and telemetry |
| openfga_sdk/sync/client/client.py | Sync version of raw_request with identical logic |
| openfga_sdk/api_client.py | Fixed header handling to return dict from getheaders() |
| openfga_sdk/sync/api_client.py | Fixed header handling in sync client |
| openfga_sdk/telemetry/attributes.py | Added type check for response.body before dict operations |
| openfga_sdk/client/models/init.py | Exported RawResponse in module |
| test/client/client_test.py | Comprehensive async tests for raw_request |
| test/sync/client/client_test.py | Comprehensive sync tests for raw_request |
| README.md | Documentation with usage examples |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| path_params_dict = dict(path_params) if path_params else {} | ||
|
|
||
| if "{store_id}" in resource_path and "store_id" not in path_params_dict: |
There was a problem hiding this comment.
Trailing whitespace on line 1148 after the dictionary assignment. Remove the extra whitespace for consistency with the codebase style.
| ) | ||
| path_params_dict["store_id"] = store_id | ||
|
|
||
| for param_name, param_value in path_params_dict.items(): |
There was a problem hiding this comment.
Trailing whitespace on line 1157 after the dictionary assignment. Remove the extra whitespace for consistency with the codebase style.
| encoded_value = urllib.parse.quote(str(param_value), safe="") | ||
| resource_path = resource_path.replace(placeholder, encoded_value) | ||
|
|
||
| if "{" in resource_path or "}" in resource_path: |
There was a problem hiding this comment.
Trailing whitespace on line 1163 after the string replacement. Remove the extra whitespace for consistency with the codebase style.
| path_params_dict = dict(path_params) if path_params else {} | ||
|
|
||
| if "{store_id}" in resource_path and "store_id" not in path_params_dict: |
There was a problem hiding this comment.
Trailing whitespace on line 1148 after the dictionary assignment. Remove the extra whitespace for consistency with the codebase style.
| placeholder = f"{{{param_name}}}" | ||
| if placeholder in resource_path: | ||
| encoded_value = urllib.parse.quote(str(param_value), safe="") | ||
| resource_path = resource_path.replace(placeholder, encoded_value) |
There was a problem hiding this comment.
Missing blank line after line 1163 before the validation check. Add a blank line for consistency with the sync client implementation and improved readability.
| resource_path = resource_path.replace(placeholder, encoded_value) | |
| resource_path = resource_path.replace(placeholder, encoded_value) |
| try: | ||
| _, http_status, http_headers = self._api_client.call_api( | ||
| resource_path=resource_path, | ||
| method=method.upper(), | ||
| query_params=query_params_list if query_params_list else None, | ||
| header_params=auth_headers if auth_headers else None, | ||
| body=body_params, | ||
| response_types_map={}, | ||
| auth_settings=[], | ||
| _return_http_data_only=False, | ||
| _preload_content=True, | ||
| _retry_params=retry_params, | ||
| _oauth2_client=self._api._oauth2_client, | ||
| _telemetry_attributes=telemetry_attributes, | ||
| ) | ||
| except ApiException as e: | ||
| raise |
There was a problem hiding this comment.
The exception handling catches ApiException only to re-raise it without modification. This catch-and-re-raise pattern serves no purpose and can be removed entirely for cleaner code.
| try: | |
| _, http_status, http_headers = self._api_client.call_api( | |
| resource_path=resource_path, | |
| method=method.upper(), | |
| query_params=query_params_list if query_params_list else None, | |
| header_params=auth_headers if auth_headers else None, | |
| body=body_params, | |
| response_types_map={}, | |
| auth_settings=[], | |
| _return_http_data_only=False, | |
| _preload_content=True, | |
| _retry_params=retry_params, | |
| _oauth2_client=self._api._oauth2_client, | |
| _telemetry_attributes=telemetry_attributes, | |
| ) | |
| except ApiException as e: | |
| raise | |
| _, http_status, http_headers = self._api_client.call_api( | |
| resource_path=resource_path, | |
| method=method.upper(), | |
| query_params=query_params_list if query_params_list else None, | |
| header_params=auth_headers if auth_headers else None, | |
| body=body_params, | |
| response_types_map={}, | |
| auth_settings=[], | |
| _return_http_data_only=False, | |
| _preload_content=True, | |
| _retry_params=retry_params, | |
| _oauth2_client=self._api._oauth2_client, | |
| _telemetry_attributes=telemetry_attributes, | |
| ) |
| try: | ||
| _, http_status, http_headers = await self._api_client.call_api( | ||
| resource_path=resource_path, | ||
| method=method.upper(), | ||
| query_params=query_params_list if query_params_list else None, | ||
| header_params=auth_headers if auth_headers else None, | ||
| body=body_params, | ||
| response_types_map={}, | ||
| auth_settings=[], | ||
| _return_http_data_only=False, | ||
| _preload_content=True, | ||
| _retry_params=retry_params, | ||
| _oauth2_client=self._api._oauth2_client, | ||
| _telemetry_attributes=telemetry_attributes, | ||
| ) | ||
| except ApiException as e: | ||
| raise |
There was a problem hiding this comment.
The exception handling catches ApiException only to re-raise it without modification. This catch-and-re-raise pattern serves no purpose and can be removed entirely for cleaner code.
| try: | |
| _, http_status, http_headers = await self._api_client.call_api( | |
| resource_path=resource_path, | |
| method=method.upper(), | |
| query_params=query_params_list if query_params_list else None, | |
| header_params=auth_headers if auth_headers else None, | |
| body=body_params, | |
| response_types_map={}, | |
| auth_settings=[], | |
| _return_http_data_only=False, | |
| _preload_content=True, | |
| _retry_params=retry_params, | |
| _oauth2_client=self._api._oauth2_client, | |
| _telemetry_attributes=telemetry_attributes, | |
| ) | |
| except ApiException as e: | |
| raise | |
| _, http_status, http_headers = await self._api_client.call_api( | |
| resource_path=resource_path, | |
| method=method.upper(), | |
| query_params=query_params_list if query_params_list else None, | |
| header_params=auth_headers if auth_headers else None, | |
| body=body_params, | |
| response_types_map={}, | |
| auth_settings=[], | |
| _return_http_data_only=False, | |
| _preload_content=True, | |
| _retry_params=retry_params, | |
| _oauth2_client=self._api._oauth2_client, | |
| _telemetry_attributes=telemetry_attributes, | |
| ) |
| ) | ||
|
|
||
| if rest_response is None: | ||
| raise RuntimeError("Failed to get response from API client") |
There was a problem hiding this comment.
The error message 'Failed to get response from API client' is vague. Consider providing more context about why this might happen and what the user should check, such as 'Failed to get response from API client. This may indicate an internal SDK error or configuration issue.'
| raise RuntimeError("Failed to get response from API client") | |
| raise RuntimeError( | |
| "Failed to get response from API client. This may indicate an internal " | |
| "SDK error or a client configuration issue (for example, authentication, " | |
| "network, or transport misconfiguration)." | |
| ) |
| ) | ||
|
|
||
| if rest_response is None: | ||
| raise RuntimeError("Failed to get response from API client") |
There was a problem hiding this comment.
The error message 'Failed to get response from API client' is vague. Consider providing more context about why this might happen and what the user should check, such as 'Failed to get response from API client. This may indicate an internal SDK error or configuration issue.'
| raise RuntimeError("Failed to get response from API client") | |
| raise RuntimeError( | |
| f"Failed to get response from API client for {method.upper()} " | |
| f"request to '{resource_path}'" | |
| f"{f' (operation: {operation_name})' if operation_name else ''}. " | |
| "This may indicate an internal SDK error, network problem, or client configuration issue." | |
| ) |
|
Thanks for your PR @kcbiradar |
2 participants
This adds the capability to call any FGA endpoint.
What problem is being solved?
How is it being solved?
What changes are made to solve it?
References
closes
Review Checklist
mainSummary by CodeRabbit
Release Notes
New Features
raw_requestmethod to OpenFgaClient for making custom HTTP requests to OpenFGA endpointsBug Fixes
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.