LCORE-792: Lightspeed core needs to fully support VertexAI LLM provider#924
Conversation
WalkthroughThis PR adds VertexAI as a supported LLM provider to the e2e testing framework and infrastructure. Changes include adding VertexAI to the CI workflow matrix, restructuring configuration file paths to use a mode-based directory structure, integrating Google Cloud credentials handling, updating Docker compose services with GCP environment variables and volume mounts, and creating new VertexAI configuration examples and test setups. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–25 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
|
e2e tests fail here because of missing file, PTAL at the test results in my fork |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
README.md (1)
179-181: Consider using descriptive link text in the footnote.The link text "here" is not descriptive, which can reduce accessibility and clarity. Consider using more meaningful text like "the llama-stack source code".
-[^1]: List of models is limited by design in llama-stack, future versions will probably allow to use more models (see [here](https://github.com/llamastack/llama-stack/blob/release-0.3.x/llama_stack/providers/remote/inference/vertexai/vertexai.py#L54)) +[^1]: List of models is limited by design in llama-stack, future versions will probably allow to use more models (see [llama-stack vertexai provider](https://github.com/llamastack/llama-stack/blob/release-0.3.x/llama_stack/providers/remote/inference/vertexai/vertexai.py#L54))
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
.github/workflows/e2e_tests.yaml(5 hunks)README.md(2 hunks)docker-compose-library.yaml(1 hunks)docker-compose.yaml(1 hunks)docs/providers.md(1 hunks)examples/vertexai-run.yaml(1 hunks)tests/e2e/configs/run-vertexai.yaml(1 hunks)tests/e2e/configuration/library-mode/lightspeed-stack.yaml(1 hunks)tests/e2e/configuration/server-mode/lightspeed-stack.yaml(1 hunks)tests/e2e/features/environment.py(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Use
pytest-mockwith AsyncMock objects for mocking in tests
Files:
tests/e2e/features/environment.py
🧠 Learnings (3)
📚 Learning: 2025-09-02T11:09:40.404Z
Learnt from: radofuchs
Repo: lightspeed-core/lightspeed-stack PR: 485
File: tests/e2e/features/environment.py:87-95
Timestamp: 2025-09-02T11:09:40.404Z
Learning: In the lightspeed-stack e2e tests, noop authentication tests use the default lightspeed-stack.yaml configuration, while noop-with-token tests use the Authorized tag to trigger a config swap to the specialized noop-with-token configuration file.
Applied to files:
tests/e2e/configuration/server-mode/lightspeed-stack.yamltests/e2e/configuration/library-mode/lightspeed-stack.yaml
📚 Learning: 2025-09-02T11:15:02.411Z
Learnt from: radofuchs
Repo: lightspeed-core/lightspeed-stack PR: 485
File: tests/e2e/test_list.txt:2-3
Timestamp: 2025-09-02T11:15:02.411Z
Learning: In the lightspeed-stack e2e tests, the Authorized tag is intentionally omitted from noop authentication tests because they are designed to test against the default lightspeed-stack.yaml configuration rather than the specialized noop-with-token configuration.
Applied to files:
tests/e2e/configuration/server-mode/lightspeed-stack.yamltests/e2e/configuration/library-mode/lightspeed-stack.yaml
📚 Learning: 2025-08-19T08:57:27.714Z
Learnt from: onmete
Repo: lightspeed-core/lightspeed-stack PR: 417
File: src/lightspeed_stack.py:60-63
Timestamp: 2025-08-19T08:57:27.714Z
Learning: In the lightspeed-stack project, file permission hardening (chmod 0o600) for stored configuration JSON files is not required as it's not considered a security concern in their deployment environment.
Applied to files:
tests/e2e/configuration/library-mode/lightspeed-stack.yaml
🪛 markdownlint-cli2 (0.18.1)
README.md
124-124: Bare URL used
(MD034, no-bare-urls)
181-181: Link text should be descriptive
(MD059, descriptive-link-text)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-pr
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
🔇 Additional comments (18)
docs/providers.md (1)
57-57: LGTM!The VertexAI provider entry is correctly added to the Inference Providers table with appropriate dependency (
google-auth) and support status (✅). This aligns with the broader PR changes introducing VertexAI support.README.md (1)
124-124: LGTM!The VertexAI provider entry is correctly added to the prerequisites table, consistent with existing entries.
docker-compose.yaml (2)
13-13: LGTM!The GCP keys volume mount is appropriately configured as read-only with a sensible default to a dummy directory for non-VertexAI environments.
31-34: TheGOOGLE_APPLICATION_CREDENTIALSenvironment variable is already correctly configured to point to the container path/opt/app-root/.gcp-keys/gcp-key.json. The docker-compose.yaml volume mount (${GCP_KEYS_PATH:-./tmp/.gcp-keys-dummy}:/opt/app-root/.gcp-keys:ro) properly maps the host credentials directory to the container path, and the e2e workflow (line 130) explicitly sets the variable to the container path before passing it to docker-compose. No changes needed.Likely an incorrect or invalid review comment.
docker-compose-library.yaml (2)
17-36: LGTM!The environment variables are consistently structured with appropriate defaults. The VertexAI variables follow the same pattern as other providers.
15-15: Verify thatGOOGLE_APPLICATION_CREDENTIALSis set to/opt/app-root/.gcp-keysin the environment configuration. While the volume mounts to a different path than the container's WORKDIR, this is not inherently problematic—what matters is that the environment variable explicitly points to the mounted credentials file path.examples/vertexai-run.yaml (4)
1-1: LGTM!The version 2 configuration schema is appropriate for the current llama-stack integration.
76-81: LGTM!The VertexAI inference provider is correctly configured with environment variable templating for
projectandlocation, which aligns with the docker-compose environment variable definitions.
111-133: LGTM!The storage configuration is well-structured with appropriate sqlite backends for both key-value and SQL stores. The store mappings for metadata, inference, conversations, and prompts are properly defined.
134-140: Remove: The emptymodelsarray is intentional and consistent with repository patterns.After examining the codebase, no other example configurations in the repository contain a
registered_resourcessection at all. The pattern invertexai-run.yaml—with all resource arrays (models, shields, vector_dbs, datasets, scoring_fns, benchmarks) empty except for tool_groups—represents an intentional default configuration template. The assumption that "other example configurations typically pre-register models" does not hold in this repository. No change or comment is required.Likely an incorrect or invalid review comment.
tests/e2e/features/environment.py (2)
58-58: Good addition - comment improves clarity.The comment effectively explains the purpose of the model detection logic that follows.
70-75: Dynamic messaging improvement approved; model change verified.The dynamic fallback message is more informative. GPT-4o Mini is roughly 53.3x cheaper compared to GPT-4 Turbo for input and output tokens, making this a sound cost optimization.
.github/workflows/e2e_tests.yaml (4)
13-13: LGTM - VertexAI added to test matrix.The addition of
vertexaito the environment matrix enables comprehensive testing across all supported providers.
52-65: LGTM - improved config path structure.The restructuring to
${MODE}-mode/lightspeed-stack.yamlprovides better organization through directory-based mode separation.
93-131: Well-implemented VertexAI credentials handling.The service account key setup includes good practices:
- Base64 encoding prevents GitHub from modifying the key
- JSON validation catches malformed keys early
- Clear error messages for debugging
The implementation correctly sets both
GOOGLE_APPLICATION_CREDENTIALSandGCP_KEYS_PATHfor downstream use.
194-240: LGTM - VertexAI environment variables properly propagated.The VertexAI-related environment variables (
VERTEX_AI_LOCATION,VERTEX_AI_PROJECT,GOOGLE_APPLICATION_CREDENTIALS) are correctly propagated to both server and library mode runs, ensuring consistent configuration across deployment modes.Note: The
GCP_KEYS_PATHissue for non-VertexAI environments is addressed in the previous comment.tests/e2e/configs/run-vertexai.yaml (2)
1-143: Well-structured VertexAI configuration.The configuration file is comprehensive and properly structured, including:
- All required API modules and providers
- Appropriate storage backend configurations
- Proper namespace and path definitions
- Complete registered resources sections
The file provides a solid foundation for VertexAI end-to-end testing.
76-81: Configuration syntax is correct.The
${env.VERTEX_AI_PROJECT}and${env.VERTEX_AI_LOCATION}syntax aligns with llama-stack's documented environment variable substitution format.
| - name: Create dummy GCP keys directory | ||
| if: matrix.environment != 'vertexai' | ||
| run: | | ||
| echo "Creating dummy GCP keys directory for non-VertexAI environment..." | ||
| mkdir -p ./tmp/.gcp-keys-dummy | ||
| echo "✅ Dummy directory created." |
There was a problem hiding this comment.
Critical: GCP_KEYS_PATH not set for non-VertexAI environments.
The dummy directory is created, but the GCP_KEYS_PATH environment variable is never set for non-VertexAI environments (ci, azure). This causes GCP_KEYS_PATH to be undefined when passed to docker-compose at lines 202 and 228, which may lead to volume mount failures.
Apply this fix:
- name: Create dummy GCP keys directory
if: matrix.environment != 'vertexai'
run: |
echo "Creating dummy GCP keys directory for non-VertexAI environment..."
mkdir -p ./tmp/.gcp-keys-dummy
+ echo "GCP_KEYS_PATH=./tmp/.gcp-keys-dummy" >> $GITHUB_ENV
echo "✅ Dummy directory created."🤖 Prompt for AI Agents
.github/workflows/e2e_tests.yaml around lines 187 to 192: the dummy GCP keys
directory is created for non-VertexAI environments but GCP_KEYS_PATH is not set,
so downstream steps (docker-compose) receive an undefined path; update the
workflow to export GCP_KEYS_PATH pointing to the created dummy directory so
subsequent steps see it (for example, append
"GCP_KEYS_PATH=./tmp/.gcp-keys-dummy" to $GITHUB_ENV or set it in the step's
env) ensuring the variable is available to later steps that use it for
docker-compose volume mounts.
There was a problem hiding this comment.
This is addressed by setting a fallback GCP_KEYS_PATH in the docker-compose.yaml
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the GCP_KEYS_PATH fallback in docker-compose files
echo "=== Checking docker-compose.yaml ==="
grep -n "GCP_KEYS_PATH" docker-compose.yaml
echo -e "\n=== Checking docker-compose-library.yaml ==="
grep -n "GCP_KEYS_PATH" docker-compose-library.yaml
Length of output: 238
@are-ces You're absolutely right! I can see both docker-compose.yaml and docker-compose-library.yaml use the syntax ${GCP_KEYS_PATH:-./tmp/.gcp-keys-dummy}, which provides a fallback value when the environment variable is unset. This means the volume mount will work correctly for non-VertexAI environments even without explicitly setting GCP_KEYS_PATH in the workflow.
My original concern was incorrect—the code already handles this case properly. Thank you for the clarification! 👍
3 participants
Description
Type of change
Tools used to create PR
Identify any AI code assistants used in this PR (for transparency and review context)
Related Tickets & Documents
Checklist before requesting a review
Testing
Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.