-
Notifications
You must be signed in to change notification settings - Fork 63
feat(dotnet-sdk): APIExecutor for RawRequests #676
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
WalkthroughThese changes enable streamed list objects support in the .NET SDK by setting a configuration flag to true and add an internal property to expose the underlying ApiClient instance for custom requests without public exposure. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR aims to expose the internal ApiClient for making custom API requests in the .NET SDK, but the implementation appears incomplete or misaligned with the stated purpose.
Changes:
- Adds an
internalpropertyApiClientInternalto expose the underlying ApiClient instance - Enables streaming support for list objects by changing
supportsStreamedListObjectsfromfalsetotrue
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| config/clients/dotnet/template/api.mustache | Adds internal property to expose ApiClient for custom requests |
| config/clients/dotnet/config.overrides.json | Enables streaming support for list objects |
| /// <summary> | ||
| /// Gets the internal ApiClient instance for making custom API requests. | ||
| /// </summary> | ||
| internal ApiClient.ApiClient ApiClientInternal => _apiClient; |
Copilot
AI
Feb 4, 2026
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.
The PR title mentions "APIExecutor for RawRequests" but there is no APIExecutor class added in this PR. The changes only expose the internal ApiClient via a new property. Either the PR title should be updated to accurately reflect what's being changed (e.g., "expose internal ApiClient for custom requests"), or additional changes are needed to implement the APIExecutor functionality mentioned in the title.
| /// Gets the internal ApiClient instance for making custom API requests. | ||
| /// </summary> |
Copilot
AI
Feb 4, 2026
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.
The PR description is empty with no explanation of what problem is being solved, how it's being solved, or what changes are made. This makes it difficult to review the changes in context and understand the intended use case for exposing the internal ApiClient. Please fill out the PR description with details about the feature being added.
| /// Gets the internal ApiClient instance for making custom API requests. | |
| /// </summary> | |
| /// Gets the internal <see cref="ApiClient.ApiClient"/> instance used by this API class. | |
| /// </summary> | |
| /// <remarks> | |
| /// This property is intended for advanced scenarios where you need to issue custom HTTP | |
| /// requests against the OpenFGA API that are not covered by the generated methods on | |
| /// this class. It reuses the same configuration and <see cref="HttpClient"/> instance | |
| /// as the generated operations, so any authentication, base URL, retry, or timeout | |
| /// settings applied to <see cref="{{classname}}"/> will also apply to requests made | |
| /// via <see cref="ApiClientInternal"/>. | |
| /// </remarks> |
| "hashCodeMultiplierPrimeNumber": 9923, | ||
| "openTelemetryDocumentation": "OpenTelemetry.md", | ||
| "supportsStreamedListObjects": false, | ||
| "supportsStreamedListObjects": true, |
Copilot
AI
Feb 4, 2026
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.
The change from false to true enables streaming support for list objects, which appears to be intentional but is not mentioned in the PR title or description. This is a separate feature change that should either be documented in the PR description or split into a separate PR. According to the Makefile (line 116-122), this flag determines whether the dotnet SDK is built with or without streaming endpoints.
Description
generates openfga/dotnet-sdk#176
What problem is being solved?
How is it being solved?
What changes are made to solve it?
References
Review Checklist
mainSummary by CodeRabbit