Skip to content

Conversation

@PsiACE
Copy link
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

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.

Signed-off-by: Chojan Shang <psiace@apache.org>
@PsiACE
Copy link
Member Author

PsiACE commented Dec 22, 2025

The failed CI appears unrelated to this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

1 participant