go-sdk
go-sdk copied to clipboard
replace errgroup with conc pool in WriteExecute method
Replace errgroup with conc pool in WriteExecute method
Description
- Replace errgroup with conc pool for parallel write/delete operations
- Use errors.As() instead of type assertion for authentication error detection
- Handle wrapped errors from conc pool using errors.As() in both client and test
- Add nil check before dereferencing singleResponse to prevent panics
- Update test to use errors.As() for proper error unwrapping
This PR only moved "Write" to use Conc
References
https://github.com/openfga/go-sdk/issues/193
Review Checklist
- [x] I have clicked on "allow edits by maintainers".
- [x] I have added documentation for new/changed functionality in this PR or in a PR to openfga.dev [Provide a link to any relevant PRs in the references section above]
- [x] The correct base branch is being used, if not
main - [x] I have added tests to validate that the change in functionality is working as expected
The committers listed above are authorized under a signed CLA.
- :white_check_mark: login: AltuisticIsopod / name: Lalit Sudhir (3915e47da00d9106bb99091c24fcd7f124b6ee53, dfeff0b3d2d025d0c25ff854939376f92138ce96)
- :white_check_mark: login: SoulPancake / name: Anurag Bandyopadhyay (1e88ab5c1920262069a1802627b3658937b543d3)
[!IMPORTANT]
Review skipped
Auto incremental reviews are disabled on this repository.
Please check the settings in the CodeRabbit UI or the
.coderabbit.yamlfile in this repository. To trigger a single review, invoke the@coderabbitai reviewcommand.You can disable this status message by setting the
reviews.review_statustofalsein the CodeRabbit configuration file.
Walkthrough
The changes replace concurrent error-group parallelism with a pool-based approach in batch write and delete operations, add four new public methods to the SdkClient interface for managing authorization model and store IDs, and improve error handling by using the errors.As package for safer type assertions across the client and test code.
Changes
| Cohort / File(s) | Summary |
|---|---|
Batch processing parallelism refactorclient/client.go |
Replaced error-group concurrent pattern with pool-based workers (pool.NewWithResults with WithContext and WithMaxGoroutines) for WriteExecute and Delete chunk processing; switched error handling to use errors.As for FgaApiAuthenticationError classification |
Public API surface expansionclient/client.go |
Added four new public methods to SdkClient interface: SetAuthorizationModelId(string) error, GetAuthorizationModelId() (string, error), SetStoreId(string) error, GetStoreId() (string, error); implemented on OpenFgaClient with ULID format validation and config storage |
Error handling improvementsclient/client.go, client/client_test.go |
Added standard errors package import and replaced direct type assertions with errors.As checks for robust FgaApiAuthenticationError detection across multiple code paths |
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20–25 minutes
The changes introduce multiple distinct patterns: a significant parallelism refactor in critical write/delete paths requiring verification of pool-worker error flow, new public API methods with straightforward validation logic, and consistent but distributed error handling improvements across two files that each require separate reasoning.
Pre-merge checks and finishing touches
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | ⚠️ Warning | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | You can run @coderabbitai generate docstrings to improve docstring coverage. |
✅ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title Check | ✅ Passed | The pull request title "replace errgroup with conc pool in WriteExecute method" directly aligns with the primary technical objective stated in the PR description. The title accurately identifies the main refactoring work: replacing the concurrent error-group approach with a conc pool-based approach for parallel operations. While the changeset includes additional modifications such as new public API methods (SetAuthorizationModelId, GetAuthorizationModelId, SetStoreId, GetStoreId) and error handling improvements using errors.As(), the title captures the core technical change that drives the majority of the code review effort. The title is specific, clear, and would enable a teammate reviewing the git history to understand the primary refactoring being performed. |
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
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 @coderabbitai help to get the list of available commands and usage tips.
- Removed manual slice management and indexing for write/delete operations
- Used conc pool's automatic result collection instead of manual assignment
Hey @rhamzeh, I have replaced errgroup with conc for Write. Can you review and let me know if you find any issues. Can you please add Hacktoberfest label to this issue? Thank you.
Hi @AltuisticIsopod, same as with https://github.com/openfga/sdk-generator/pull/653, we'll assign someone to review your PR. Thanks.
Codecov Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 33.88%. Comparing base (8ebd110) to head (3915e47).
:x: Your project status has failed because the head coverage (33.88%) 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 #249 +/- ##
==========================================
- Coverage 33.90% 33.88% -0.03%
==========================================
Files 111 111
Lines 10614 10610 -4
==========================================
- Hits 3599 3595 -4
Misses 6663 6663
Partials 352 352
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.