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
Summary by CodeRabbit
- Refactor
- Enhanced concurrent request processing with improved error handling and resource management for better reliability and performance.
Walkthrough
Replaces per-chunk parallel execution from an errgroup-based approach to a pool-based concurrency model in the Go client template. Introduces controlled worker pools with context propagation, refactors authentication error handling via errors.As, and updates WriteExecute and DeleteExecute methods to iterate over chunks through pool tasks.
Changes
| Cohort / File(s) | Summary |
|---|---|
Go Client Concurrency Refactoring config/clients/go/template/client/client.mustache |
Replaces errgroup-based per-chunk parallelism with worker pools (writePool/deletePool); adds errors package import for error inspection; converts authentication error handling from type assertion to errors.As; refactors WriteExecute and DeleteExecute to collect per-chunk responses via pool tasks instead of explicit index-based errgroup capturing; shifts result aggregation from errgroup.Wait() to pool.Wait() |
Sequence Diagram(s)
sequenceDiagram
autonumber
participant Client
participant OldErrGroup as Old: ErrGroup
participant NewPool as New: Worker Pool
participant Chunk
Client->>OldErrGroup: WriteExecute(chunks)
loop Per Chunk (indexed)
OldErrGroup->>Chunk: Go(i) → execute chunk[i]
Chunk-->>OldErrGroup: response
end
OldErrGroup->>OldErrGroup: Wait()
OldErrGroup-->>Client: combined result + error
Client->>NewPool: WriteExecute(chunks)
rect rgb(200, 220, 250)
Note over NewPool: Initialize pool with<br/>max goroutines
end
loop Per Chunk (no index capture)
NewPool->>Chunk: Go(task) → execute chunk
Chunk-->>NewPool: response (collected)
end
NewPool->>NewPool: Wait()
NewPool-->>Client: combined result + error
rect rgb(240, 200, 200)
Note over NewPool: Auth errors via errors.As<br/>instead of type assertion
end
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~25 minutes
- Concurrency pattern shift: Review the pool initialization, goroutine limit configuration, and correctness of chunk iteration and response collection logic
- Error handling refactoring: Verify
errors.Ascorrectly replaces the previous type assertion for authentication errors and maintains existing error semantics - Response aggregation: Ensure per-chunk responses are properly accumulated and the final combined result construction remains functionally equivalent to the errgroup approach
- Context propagation: Confirm context is correctly threaded through pool tasks and cleanup behavior is preserved
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 |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title Check | ✅ Passed | The pull request title "replace errgroup with conc pool in WriteExecute method" accurately describes the primary refactoring in the changeset. The title is specific and clear, correctly identifying the main change as a replacement of the concurrency mechanism from errgroup to a pool-based approach in the WriteExecute method. While the changes also include error handling improvements (switching to errors.As) and nil checks, the core objective and most significant change is the concurrency model replacement, which is properly captured in the title. The title is neither vague nor misleading, and it provides sufficient clarity for a developer scanning the repository history to understand the nature of this change. |
✨ Finishing touches
🧪 Generate unit tests (beta)
- [ ] Create PR with unit tests
- [ ] Post copyable unit tests in a comment
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.
Hi @AltuisticIsopod, we'll assign someone to review your PR. Thanks.