-
Notifications
You must be signed in to change notification settings - Fork 296
feat: Add aibrix profile for E2E testing framework #688
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
Conversation
✅ Deploy Preview for vllm-semantic-router ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
👥 vLLM Semantic Team NotificationThe following members have been identified for the changed files in this PR and have been automatically assigned: 📁
|
d29de37 to
294e4df
Compare
294e4df to
e13f277
Compare
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 a new AIBrix profile to the E2E testing framework to enable testing of Semantic Router with vLLM AIBrix integration. The implementation follows the existing pattern established by the ai-gateway profile while adding robust error handling and cleanup mechanisms.
Key changes:
- Implemented 5-stage deployment for AIBrix (Semantic Router, AIBrix Dependencies, AIBrix Core, Demo LLM, environment verification)
- Added comprehensive error handling with partial cleanup and panic recovery
- Integrated into CI with matrix strategy for parallel testing of both ai-gateway and aibrix profiles
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| e2e/profiles/aibrix/profile.go | New AIBrix profile implementation with 5-stage deployment, error handling, and cleanup logic |
| e2e/cmd/e2e/main.go | Registered the new aibrix profile in the profile factory |
| tools/make/e2e.mk | Updated help text to document the aibrix profile |
| e2e/README.md | Updated documentation to list aibrix as a supported profile (moved from future to current) |
| .github/workflows/integration-test-k8s.yml | Added matrix strategy to test both profiles in parallel with separate artifact names |
Comments suppressed due to low confidence (2)
.github/workflows/integration-test-k8s.yml:28
- Go version '1.24' does not exist as of January 2025. The latest stable Go version is 1.23. This should be corrected to a valid Go version like '1.23' or '1.22'.
go-version: '1.24'
.github/workflows/integration-test-k8s.yml:33
- Rust version 1.90 does not exist as of January 2025. The latest stable Rust version is around 1.84. This should be corrected to a valid Rust version or use 'stable' instead.
toolchain: 1.90
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
76ddf31 to
931c983
Compare
|
@yehudit1987 this is ready to go, can you resolve the conflict so we can run the latest e2e test. Thanks |
Signed-off-by: Yehudit Kerido <[email protected]>
931c983 to
749431c
Compare
|
reading the log, PII policy is disabled and that caused the test failure |
Co-authored-by: Copilot <[email protected]> Signed-off-by: yehudit1987 <[email protected]>
749431c to
a4a5e93
Compare
All tests are passing now.
|
|
thanks @yehudit1987 let run the latest code for one last time |
Xunzhuo
left a comment
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.
thanks a lot, nice work! would you like to try adding more testcases to make sure the new signal-decsion engines works well?
Hi @Xunzhuo sure, I will and thanks for the feedback. Do we have an open issue for that? Want me to create one? |
…m-project#688 The aibrix E2E profile was designed to use ModernBERT for PII detection (as configured in PR vllm-project#688), but during the merge it was incorrectly changed to use the LoRA PII model which isn't available in the aibrix deployment. This commit reverts the aibrix PII model configuration to use: - model_id: "models/pii_classifier_modernbert-base_presidio_token_model" - use_modernbert: true This restores the original working configuration from PR vllm-project#688 and allows the aibrix E2E tests to pass while keeping the LoRA PII auto-detection feature available for other profiles that explicitly configure it. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> Signed-off-by: Yossi Ovadia <[email protected]>
The previous policy.go changes added a default_decision fallback mechanism that broke aibrix profile. Aibrix uses static YAML config and doesn't have a default_decision defined. Reverting to the upstream version from PR vllm-project#688 that passed aibrix tests successfully. Related to vllm-project#648 Signed-off-by: Yossi Ovadia <[email protected]>
…m-project#688 The aibrix E2E profile was designed to use ModernBERT for PII detection (as configured in PR vllm-project#688), but during the merge it was incorrectly changed to use the LoRA PII model which isn't available in the aibrix deployment. This commit reverts the aibrix PII model configuration to use: - model_id: "models/pii_classifier_modernbert-base_presidio_token_model" - use_modernbert: true This restores the original working configuration from PR vllm-project#688 and allows the aibrix E2E tests to pass while keeping the LoRA PII auto-detection feature available for other profiles that explicitly configure it. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> Signed-off-by: Yossi Ovadia <[email protected]>

Add AIBrix profile implementing 5-stage deployment:
Implement robust error handling:
Add configurable AIBrix version via AIBRIX_VERSION env var
Integrate into CI with matrix strategy for parallel testing
Support all test cases: chat-completions, domain-classify,
semantic-cache, pii-detection, jailbreak-detection, stress tests
Resolve [E2E] Add aibrix profile for E2E testing framework #660