Skip to content

feat(layers/concurrent-limit): accept custom semaphore without API break#7082

Merged
PsiACE merged 5 commits intomainfrom
port-6618
Jan 20, 2026
Merged

feat(layers/concurrent-limit): accept custom semaphore without API break#7082
PsiACE merged 5 commits intomainfrom
port-6618

Conversation

@PsiACE
Copy link
Copy Markdown
Member

@PsiACE PsiACE commented Dec 22, 2025

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?

  • Introduce ConcurrentLimitSemaphore abstraction (layer-named) with default Arcmea::Semaphore implementation.
  • Keep ConcurrentLimitLayer::new(permits) intact; add with_semaphore(...) for shared/custom semaphore; align HTTP setter naming.
  • Add tests for shared/custom semaphore usage.

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.

Signed-off-by: Chojan Shang <psiace@apache.org>
Copilot AI review requested due to automatic review settings December 22, 2025 07:53
@PsiACE PsiACE requested a review from Xuanwo as a code owner December 22, 2025 07:53
@dosubot dosubot Bot added size:L This PR changes 100-499 lines, ignoring generated files. releases-note/feat The PR implements a new feature or has a title that begins with "feat" labels Dec 22, 2025
Copy link
Copy Markdown

Copilot AI left a 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 ConcurrentLimitSemaphore trait to abstract semaphore implementations
  • Added with_semaphore() and with_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.

Comment thread core/layers/concurrent-limit/src/lib.rs
Comment thread core/layers/concurrent-limit/src/lib.rs
Comment thread core/layers/concurrent-limit/src/lib.rs Outdated
Comment thread core/layers/concurrent-limit/src/lib.rs Outdated
Comment thread core/layers/concurrent-limit/src/lib.rs Outdated
Signed-off-by: Chojan Shang <psiace@apache.org>
@PsiACE
Copy link
Copy Markdown
Member Author

PsiACE commented Dec 22, 2025

The failed CI appears unrelated to this PR.

Copy link
Copy Markdown
Member

@Xuanwo Xuanwo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for working on this!

@dosubot dosubot Bot added the lgtm This PR has been approved by a maintainer label Jan 19, 2026
@Xuanwo
Copy link
Copy Markdown
Member

Xuanwo commented Jan 19, 2026

The only thing left is to resolve the conflicts, would you like to take another look?

@PsiACE PsiACE merged commit b23593e into main Jan 20, 2026
379 of 380 checks passed
@PsiACE PsiACE deleted the port-6618 branch January 20, 2026 03:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm This PR has been approved by a maintainer releases-note/feat The PR implements a new feature or has a title that begins with "feat" size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

new feature: ConcurrentLimitLayer should accept a Semaphore

3 participants