fix(dotnet): make Makefile respect supportsStreamedListObjects config…#662
fix(dotnet): make Makefile respect supportsStreamedListObjects config…#662daniel-jonathan merged 2 commits intomainfrom
Conversation
… flag Previously, the dotnet SDK always built with streaming support enabled, ignoring the supportsStreamedListObjects config flag. The Makefile unconditionally called build-client-streamed which hardcoded the x-streaming flag in the OpenAPI spec. Changes: - Modified build-client-dotnet to check config flag value - Added build-client-non-streamed target for non-streaming builds - Added build-openapi-non-streamed to remove streaming endpoint from spec - Default remains false since SendStreamingRequestAsync is not yet implemented The flag now works correctly - when set to false, the streaming endpoint is completely removed from the generated SDK.
|
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 WalkthroughThis PR introduces conditional build pathways for .NET clients and OpenAPI specifications, allowing the build system to generate either streamed or non-streamed variants based on configuration. New Makefile targets support non-streamed builds, while the existing build-client-dotnet target gains conditional logic to select the appropriate pathway. The dotnet config override disables streaming support. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Makefile(3 hunks)config/clients/dotnet/config.overrides.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
config/clients/*/config.overrides.json
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
config/clients/*/config.overrides.json: Always update the packageVersion in each language-specific config.overrides.json when making version changes
Maintain FOSSA compliance notice IDs in each language’s config.overrides.json
Files:
config/clients/dotnet/config.overrides.json
config/**/*.{json,mustache}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Never hardcode API keys or credentials in configuration or template files
Files:
config/clients/dotnet/config.overrides.json
config/{common/config.base.json,clients/*/config.overrides.json}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Ensure consistent versioning across base and language override configuration files to avoid version conflicts
Files:
config/clients/dotnet/config.overrides.json
Makefile
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Makefile: Update OPEN_API_REF in the Makefile when targeting new OpenFGA API versions
Update Docker image tags in the Makefile when upgrading build tools
Use official, tagged Docker images for containerized builds
Files:
Makefile
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
Repo: openfga/sdk-generator PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-04T17:35:34.111Z
Learning: Applies to scripts/build_client.sh : Implement and maintain OpenAPI transformations in scripts/build_client.sh (remove streaming endpoints, rename Object to FgaObject, strip v1. prefixes) when API schemas change
📚 Learning: 2025-09-04T17:35:34.111Z
Learnt from: CR
Repo: openfga/sdk-generator PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-04T17:35:34.111Z
Learning: Applies to config/clients/*/config.overrides.json : Maintain FOSSA compliance notice IDs in each language’s config.overrides.json
Applied to files:
config/clients/dotnet/config.overrides.json
📚 Learning: 2025-09-04T17:35:34.111Z
Learnt from: CR
Repo: openfga/sdk-generator PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-04T17:35:34.111Z
Learning: Applies to config/clients/*/config.overrides.json : Always update the packageVersion in each language-specific config.overrides.json when making version changes
Applied to files:
config/clients/dotnet/config.overrides.json
📚 Learning: 2025-09-04T17:35:34.111Z
Learnt from: CR
Repo: openfga/sdk-generator PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-04T17:35:34.111Z
Learning: Applies to scripts/build_client.sh : Implement and maintain OpenAPI transformations in scripts/build_client.sh (remove streaming endpoints, rename Object to FgaObject, strip v1. prefixes) when API schemas change
Applied to files:
Makefile
📚 Learning: 2025-09-04T17:35:34.111Z
Learnt from: CR
Repo: openfga/sdk-generator PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-04T17:35:34.111Z
Learning: Applies to Makefile : Update OPEN_API_REF in the Makefile when targeting new OpenFGA API versions
Applied to files:
Makefile
📚 Learning: 2025-09-04T17:35:34.111Z
Learnt from: CR
Repo: openfga/sdk-generator PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-04T17:35:34.111Z
Learning: Applies to Makefile : Update Docker image tags in the Makefile when upgrading build tools
Applied to files:
Makefile
📚 Learning: 2025-09-04T17:35:34.111Z
Learnt from: CR
Repo: openfga/sdk-generator PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-04T17:35:34.111Z
Learning: Applies to Makefile : Use official, tagged Docker images for containerized builds
Applied to files:
Makefile
📚 Learning: 2025-09-04T17:35:34.111Z
Learnt from: CR
Repo: openfga/sdk-generator PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-04T17:35:34.111Z
Learning: Applies to scripts/build_client.sh : Clean up temporary directories and containers during/after builds to manage resources efficiently
Applied to files:
Makefile
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build-and-test-dotnet-sdk
- GitHub Check: build-and-test-java-sdk
- GitHub Check: build-and-test-go-sdk
🔇 Additional comments (3)
config/clients/dotnet/config.overrides.json (1)
40-40: Configuration flag properly disables streaming support.The addition of
"supportsStreamedListObjects": falsecorrectly aligns with the PR's goal of enabling non-streamed builds for the .NET SDK. The change is minimal, maintains existing version and FOSSA compliance information, and requires no additional updates per the coding guidelines.Makefile (2)
110-116: Verify Docker image tag override rationale.Both build paths override
OPENAPI_GENERATOR_CLI_DOCKER_TAGfrom the defaultv6.4.0tov7.11.0. The PR objectives do not explain this version upgrade, and per the coding guidelines, tool version changes in the Makefile should be documented.Please clarify:
- Why is v7.11.0 specifically required for .NET (streamed and non-streamed)?
- Does the upstream openapi-generator v7.11.0 provide critical features or fixes needed for this PR?
- Should this version change apply to other SDK languages as well, or is it .NET-specific?
You can reference the generator's changelog to confirm the rationale: https://github.com/OpenAPITools/openapi-generator/releases
208-211: New build-client-non-streamed target structure is sound.The target properly mirrors the
build-client-streamedpattern, correctly depends onbuild-openapi-non-streamed, and appropriately delegates to./scripts/build_client.sh. The.PHONYdeclaration is present.
Address CodeRabbit feedback to also remove StreamedListObjectsResponse and StreamResultOfStreamedListObjectsResponse definitions from the non-streamed OpenAPI spec, not just the endpoint path. This ensures no orphaned model definitions remain when streaming support is disabled.
Fix dotnet SDK Makefile to Respect
supportsStreamedListObjectsConfig FlagProblem
The
supportsStreamedListObjectsconfiguration flag inconfig/clients/dotnet/config.overrides.jsonwas being ignored. The Makefile always built the dotnet SDK with streaming support enabled, causing build failures:Root Cause
The
build-client-dotnettarget unconditionally calledbuild-client-streamed, which hardcodedx-streaming = truein the OpenAPI spec regardless of the config flag setting.Solution
Modified the Makefile to check the config flag and conditionally build with or without streaming support:
build-client-dotnetto check the config flag valuebuild-client-non-streamedtarget for builds without streamingbuild-openapi-non-streamedtarget that removes the/streamed-list-objectsendpoint from the OpenAPI specChanges
Result
The flag now works correctly:
supportsStreamedListObjects: false(current default) - streaming endpoint is not generated, build succeedssupportsStreamedListObjects: true(future) - streaming endpoint is generatedConfiguration
Default is set to
falseinconfig/clients/dotnet/config.overrides.jsonsinceSendStreamingRequestAsync()is not yet implemented.Impact
This fix is dotnet-specific and does not affect other SDKs (Python, Go, Java, JS).
Testing
Files Modified
sdk-generator/MakefileSummary by CodeRabbit