-
Notifications
You must be signed in to change notification settings - Fork 697
feat(layers/concurrent-limit): accept custom semaphore without API break #7082
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
Signed-off-by: Chojan Shang <psiace@apache.org>
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 adds support for custom/shared semaphores to ConcurrentLimitLayer without breaking the existing API. It introduces a new ConcurrentLimitSemaphore trait abstraction that allows users to provide their own semaphore implementations while maintaining backward compatibility with the existing new(permits) constructor.
Key changes:
- Introduced
ConcurrentLimitSemaphoretrait to abstract semaphore implementations - Added
with_semaphore()andwith_http_semaphore()methods for custom semaphore injection - Refactored internal types to be generic over the semaphore implementation
- Added tests demonstrating shared and custom semaphore usage
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
core/layers/concurrent-limit/src/lib.rs |
Core implementation: added ConcurrentLimitSemaphore trait, made layer/accessor/wrapper types generic over semaphore type, added new API methods, and included tests for shared and custom semaphores |
core/layers/concurrent-limit/Cargo.toml |
Added tokio as dev-dependency for async test support |
core/Cargo.lock |
Updated lock file to reflect new tokio dev-dependency |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Chojan Shang <psiace@apache.org>
|
The failed CI appears unrelated to this PR. |
Which issue does this PR close?
Closes #6591 .
Port #6618 with fixes.
Rationale for this change
Add support for passing a shared/custom semaphore to ConcurrentLimitLayer without breaking existing APIs.
What changes are included in this PR?
Are there any user-facing changes?
Yes. ConcurrentLimitLayer can now accept a custom/shared semaphore via with_semaphore(...) while keeping the existing new(permits) constructor.
AI Usage Statement
Implemented with OpenAI Codex (GPT-5) via Codex CLI.