feat: enhance integration testing workflow for Vertex AI#136
feat: enhance integration testing workflow for Vertex AI#136Artemon-line wants to merge 1 commit intoopendatahub-io:mainfrom
Conversation
- Added a step to check for Vertex AI recordings in the integration tests workflow, allowing conditional execution of Vertex AI tests based on the presence of recordings. - Updated the integration tests script to dynamically select the text model based on the environment, supporting both Vertex AI and vllm-inference providers. This improves the testing process by enabling the use of recorded data for Vertex AI, ensuring more reliable and efficient test runs. Signed-off-by: Artemy <ahladenk@redhat.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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 |
|
@Artemon-line can you refrain from adding reviewers until the PR is out of draft? |
| run: | | ||
| docker run -d --net=host -p 8321:8321 \ | ||
| -v "$HOME/.config/gcloud:/root/.config/gcloud:ro" \ | ||
| -e VERTEX_AI_PROJECT="${{ env.VERTEX_AI_PROJECT }}" \ |
There was a problem hiding this comment.
Why don't we use secrets.VERTEX_AI_PROJECT directly here? There is a level of indirection that makes this more difficult to follow than necessary IMHO
| - name: Set Vertex AI environment variables | ||
| run: | | ||
| echo "VERTEX_AI_PROJECT=${{ secrets.VERTEX_AI_PROJECT }}" >> $GITHUB_ENV | ||
| echo "VERTEX_AI_LOCATION=${{ secrets.VERTEX_AI_LOCATION || 'us-central1' }}" >> $GITHUB_ENV |
There was a problem hiding this comment.
VERTEX_AI_LOCATION is not a secret. We could just hardcode it to us-central1 for simplicity.
| **Source**: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} | ||
| **Date**: $(date -u +%Y-%m-%d) | ||
|
|
||
| Auto-generated by the live CI tests workflow." \ |
There was a problem hiding this comment.
We're going to have a lot of commits with this message. You can delete this Auto-generated line - it's just repetitive noise since we're recording Source and Date.
|
|
||
| ## Quick Start | ||
|
|
||
| ### 1. Install `act` |
There was a problem hiding this comment.
This section is duplicated with above.
| podman ps | ||
| ``` | ||
|
|
||
| **Note**: The helper script (`scripts/test-with-act.sh`) will automatically detect and use Podman if available, falling back to Docker if needed. GitHub Actions workflows use Docker, but Podman is preferred for local testing. |
There was a problem hiding this comment.
This sentence "GitHub Actions workflows use Docker, but Podman is preferred for local testing." is duplicated several times.
|
|
||
| ### 2. Self-Hosted Actions | ||
|
|
||
| Actions in `.github/actions/` need to be available. `act` should handle this automatically. |
There was a problem hiding this comment.
I don't understand the purpose of this section
|
|
||
| 1. **Podman** - Required for building and running containers (including vllm container) | ||
| 2. **gcloud CLI** - For GCP authentication (if using Vertex AI provider) | ||
| 3. **Git** - For cloning repositories |
There was a problem hiding this comment.
These documents should assume basic things like Git or Python are installed.
| #### Vertex AI | ||
| - `VERTEX_AI_PROJECT` - GCP project ID (required) | ||
| - `VERTEX_AI_LOCATION` - GCP region (optional, defaults to us-central1) | ||
| - GCP authentication via `gcloud auth application-default login` |
There was a problem hiding this comment.
This information is duplicated with above.
| gcloud auth application-default login | ||
|
|
||
| # Verify project is set | ||
| echo $VERTEX_AI_PROJECT |
There was a problem hiding this comment.
The shell script can't do this automatically for us? Can we take this out of the .md file that humans read?
| uses: google-github-actions/auth@v2 | ||
| with: | ||
| workload_identity_provider: ${{ secrets.GCP_WORKLOAD_IDENTITY_PROVIDER }} | ||
| service_account: ${{ secrets.GCP_SERVICE_ACCOUNT }} |
There was a problem hiding this comment.
We're using the procedure documented in the section "(Preferred) Direct Workload Identity Federation". There is no service account - the service_account parameter here is unnecessary.
|
@Artemon-line I suggest that you push your changes to a work-in-progress branch in this When we're happy with how this is working on that branch, then we can create the PR to |
Thanks a lot @ktdreyer, I will follow up on all the findings you highlighted and also redo some stuff to make it less robust. I understand your point about using the repo branch, which is my next step; this one was just for an overall introduction to the idea of reusing recordings. |
This improves the testing process by enabling the use of recorded data for Vertex AI, ensuring more reliable and efficient test runs.
What does this PR do?
Test Plan