Skip to content
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.

Sign up for GitHub

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

VertexAI: add support for mock responses versioning system #13206

Closed
wants to merge 2 commits into from
Closed

VertexAI: add support for mock responses versioning system #13206

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
55313a6
VertexAI: clone specific version of mock responses
tanzimfh Jun 29, 2024
9c3485a
add PR comment workflow
tanzimfh Jun 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions .github/workflows/vertexai.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,41 @@ concurrency:
cancel-in-progress: true

jobs:
check-mock-responses-version:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- name: Clone mock responses
run: scripts/update_vertexai_responses.sh
- name: Find cloned and latest versions
run: |
echo "current_tag=$(git describe --tags | awk -F'/' '{print $NF}')" >> $GITHUB_ENV
Copy link
Contributor

Choose a reason for hiding this comment

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

When running this command I'm getting

v0.8.0-47-g2bfe9e3

in a misc repo. This happens when the latest commit isn't annotated with a tag.

This would make the equality check below never pass.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe the commit that git describe sees will always be annotated with a tag since update_vertexai_responses.sh clones a specific tag.

For example, the current latest commit in vertexai-sdk-test-data is not annotated with a tag, but cloning with

git clone --depth 1 --branch v1.0 https://github.com/FirebaseExtended/vertexai-sdk-test-data

and running git describe --tags shows v1.0.

Copy link
Contributor

Choose a reason for hiding this comment

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

Then you only need git tag -l as there's only one tag available, right?

echo "latest_tag=$(git -c 'versionsort.suffix=-' ls-remote --tags --sort='v:refname' https://github.com/FirebaseExtended/vertexai-sdk-test-data.git | tail -n1 | awk -F'/' '{print $NF}')" >> $GITHUB_ENV
Copy link
Contributor

Choose a reason for hiding this comment

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

Why you need the last awk command?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The output before awk includes the commit hash and looks like this:

0b55b397d27706407eb4062dd70f18cad47edcfc        refs/tags/v1.0

I'm using the awk command to extract the tag v1.0 so that it can be compared with the output of git describe --tags.

But I see that the awk command above with git describe --tags is unnecessary.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, thanks. I meant to comment on the command above.

working-directory: FirebaseVertexAI/Tests/Unit/vertexai-sdk-test-data
- name: Find comment from previous run if exists
uses: peter-evans/find-comment@v3
Copy link
Contributor

Choose a reason for hiding this comment

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

since this isn't an official github action, could you pin to a commit rather to the tag?, same below

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Andrew is looking into whether we can use unofficial GitHub actions at all. I'll pin it to a commit if we end up using this action. Thanks!

id: fc
with:
issue-number: ${{github.event.number}}
body-includes: Mock Responses Check
- name: Comment on PR if newer version is available
if: ${{env.current_tag != env.latest_tag && !steps.fc.outputs.comment-id}}
uses: peter-evans/create-or-update-comment@v4
with:
issue-number: ${{github.event.number}}
body: |
### Vertex AI Mock Responses Check :warning:
A newer major version of the mock responses for Vertex AI unit tests is available.
[update_vertexai_responses.sh](https://github.com/firebase/firebase-ios-sdk/blob/main/scripts/update_vertexai_responses.sh) should be updated to clone the latest version of the responses.
- name: Fail job if newer version is available
Copy link
Contributor

Choose a reason for hiding this comment

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

@andrewheard do you want the test to fail if not using the latest version? IMO just warning should be enough as to not block changes to other sdks

if: ${{env.current_tag != env.latest_tag}}
run: exit 1
- name: Delete comment when version gets updated
if: ${{env.current_tag == env.latest_tag && steps.fc.outputs.comment-id}}
uses: detomarco/[email protected]
with:
comment-id: ${{ steps.fc.outputs.comment-id }}

spm-unit:
strategy:
matrix:
Expand Down
6 changes: 5 additions & 1 deletion scripts/update_vertexai_responses.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@
# This script replaces mock response files for Vertex AI unit tests with a fresh
# clone of the shared repository of Vertex AI test data.

RESPONSES_VERSION='v1.*' # the major version of mock responses to use
REPO="https://github.com/FirebaseExtended/vertexai-sdk-test-data.git"
TAG=$(git -c 'versionsort.suffix=-' ls-remote --tags --sort='v:refname' "$REPO" | grep "$RESPONSES_VERSION" | tail -n1 | awk -F'/' '{print $NF}')
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a comment here to explain what the command does

Copy link
Contributor

Choose a reason for hiding this comment

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

Does the output include any / ? it's not clear either why the awk is needed here.


cd "$(dirname "$0")/../FirebaseVertexAI/Tests/Unit" || exit
rm -rf vertexai-sdk-test-data || exit
git clone --depth 1 https://github.com/FirebaseExtended/vertexai-sdk-test-data.git
git clone --depth 1 --branch "$TAG" "$REPO"
Loading