Skip to content

Clarify that key_suffix is not scoped to each strategy#221

Merged
ixti merged 3 commits intoixti:mainfrom
CJStadler:overlapping-keys
Jan 20, 2026
Merged

Clarify that key_suffix is not scoped to each strategy#221
ixti merged 3 commits intoixti:mainfrom
CJStadler:overlapping-keys

Conversation

@CJStadler
Copy link
Contributor

This PR updates the README example showing multiple concurrency strategies to make sure they generate different key suffixes. Previously the example was misleading because the calculated key suffixes could overlap, causing surprising behavior.

The example says it is intended to

Allow maximum 10 concurrent jobs per project at a time and maximum 2 jobs per user

But if there were two jobs running with project_id 1 then all jobs for user_id 1 would also be throttled, because there would already be two jobs under the key 1. This is probably not the intended behavior.

This PR also adds a spec to demonstrate the behavior.

@ixti do you have any other suggestions on how to clarify this behavior? Or do you think it should be considered a bug?

@codecov
Copy link

codecov bot commented Oct 23, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.97%. Comparing base (854dffe) to head (259670b).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #221   +/-   ##
=======================================
  Coverage   98.97%   98.97%           
=======================================
  Files          19       19           
  Lines         486      486           
  Branches       85       85           
=======================================
  Hits          481      481           
  Misses          5        5           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Owner

@ixti ixti left a comment

Choose a reason for hiding this comment

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

The documentation was wrong, indeed. Thank you for the PR and the fix.

@ixti ixti merged commit 09b3704 into ixti:main Jan 20, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants