Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughThe X-API-KEY header in the tenant authentication flow now passes the raw API token directly instead of prefixing it with "ApiKey ". Corresponding test expectations are updated to match this change. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related issues
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
backend/app/tests/test_deps_multitenant.py (1)
112-143:test_validate_multitenant_key_accepts_raw_header_valueis a near-duplicate oftest_validate_multitenant_key_parses_credentials_shape.Both test the happy path with the same assertions (URL,
X-API-KEYheader value, timeout, returnedTenantContext). The only difference is the token string ("No3x47A5"vs"abc123") and the hardcoded auth URL. This adds no additional coverage over the existing test.Consider removing this test or replacing it with a more focused assertion (e.g., testing a token that contains spaces or special characters to exercise stripping behavior) to avoid test suite bloat.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/tests/test_deps_multitenant.py` around lines 112 - 143, The test test_validate_multitenant_key_accepts_raw_header_value is a near-duplicate of test_validate_multitenant_key_parses_credentials_shape and should be removed or changed; either delete this redundant test, or modify it to exercise different behavior of validate_multitenant_key (for example supply a token with leading/trailing spaces or special characters to verify stripping/encoding behavior and assert the X-API-KEY header and TenantContext accordingly), referencing the test names test_validate_multitenant_key_accepts_raw_header_value, test_validate_multitenant_key_parses_credentials_shape and the validate_multitenant_key function so you update the right tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/app/tests/test_deps_multitenant.py`:
- Line 45: Update the test assertions that check
captured["headers"]["X-API-KEY"] to expect the header with the "ApiKey " prefix
(e.g., "ApiKey abc123" and "ApiKey No3x47A5") so they match the corrected
deps.py behavior; change the two failing assertions (the one showing
captured["headers"]["X-API-KEY"] == "abc123" and the similar assertion around
the other token) to assert the prefixed values instead.
---
Nitpick comments:
In `@backend/app/tests/test_deps_multitenant.py`:
- Around line 112-143: The test
test_validate_multitenant_key_accepts_raw_header_value is a near-duplicate of
test_validate_multitenant_key_parses_credentials_shape and should be removed or
changed; either delete this redundant test, or modify it to exercise different
behavior of validate_multitenant_key (for example supply a token with
leading/trailing spaces or special characters to verify stripping/encoding
behavior and assert the X-API-KEY header and TenantContext accordingly),
referencing the test names
test_validate_multitenant_key_accepts_raw_header_value,
test_validate_multitenant_key_parses_credentials_shape and the
validate_multitenant_key function so you update the right tests.
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
Summary
Target issue is #56
Explain the motivation for making this change. What existing problem does the pull request solve?
When guardrails calls kaapi-backend for ban lists API, we use X-API-KEY header.
For X-API-KEY header in guardrail repo, we should take the entire API key like "ApiKey " and not directly.
Checklist
Before submitting a pull request, please ensure that you mark these task.
fastapi run --reload app/main.pyordocker compose upin the repository root and test.Notes
Please add here if any other information is required for the reviewer.
Summary by CodeRabbit