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

Jump to bottom

[Vertex AI] Test SDK with v1 API instead of v1beta #14345

Closed
wants to merge 3 commits into from
Closed

[Vertex AI] Test SDK with v1 API instead of v1beta #14345

wants to merge 3 commits into from

Conversation

andrewheard
Copy link
Contributor

Do not merge. Testing the Vertex AI SDK with the v1 version of the API (instead of v1beta).

#no-changelog

@andrewheard
Copy link
Contributor Author

/gemini summary

@gemini-code-assist gemini-code-assist
Copy link
Contributor

Summary of Changes

This pull request is for testing the Vertex AI SDK with the v1 version of the API instead of the v1beta version. It modifies the base URL and API version used in the SDK, and adds new integration tests to verify the functionality with the v1 API. The changes include updating the Constants.swift file to use a staging base URL, updating the GenerativeAIRequest.swift file to use the v1 API version, and adding new tests to IntegrationTests.swift to test generateContent and generateContentStream with images and different authentication scenarios.

Highlights

  • API Version Update: The API version used by the SDK has been updated from v1beta to v1 in GenerativeAIRequest.swift.
  • Base URL Change: The base URL for the Vertex AI backend has been updated to a staging environment in Constants.swift.
  • New Integration Tests: New integration tests have been added to IntegrationTests.swift to test image generation and streaming, as well as permission scenarios with authenticated users.

Changelog

  • FirebaseVertexAI/Sources/Constants.swift
    • Updated the base URL to use the staging environment: https://staging-firebasevertexai.sandbox.googleapis.com.
  • FirebaseVertexAI/Sources/GenerativeAIRequest.swift
    • Updated the API version to v1.
  • FirebaseVertexAI/Tests/TestApp/Tests/Integration/IntegrationTests.swift
    • Added testGenerateContent_text() to test text generation.
    • Added testGenerateContent_image_fileData_public() to test image generation with public file data.
    • Added testGenerateContent_imageData_requiresAuth_wrongUser_permissionDenied() to test image generation with authenticated file data and permission errors.
    • Modified testGenerateContent_appCheckNotConfigured_shouldFail() to check the response instead of just not failing.
    • Added testGenerateContentStream_text() to test text generation streaming.
    • Added testGenerateContentStream_image_fileData_public() to test image generation streaming with public file data.
    • Added testGenContentStream_image_fileData_requiresAuth_wrongUser_permissionDenied() to test image generation streaming with authenticated file data and permission errors.
    • Modified testCountTokens_text() to check the response instead of just not failing.
    • Modified testCountTokens_image_fileData_requiresAuth_wrongUser_permissionDenied() to check the response instead of just not failing.

Activity

  • andrewheard requested a summary of the pull request using /gemini summary.

@andrewheard
Copy link
Contributor Author

/gemini review

gemini-code-assist[bot]
gemini-code-assist bot reviewed Feb 26, 2025
View reviewed changes
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the Vertex AI SDK to use the v1 API instead of v1beta. The changes include updating the base URL and API version, as well as adding new integration tests. The code appears to be well-written and the changes are straightforward. I have provided some minor feedback in the review comments. Please address these comments and have others review and approve this code before merging.

Summary of Findings

  • Test Naming: Consider renaming the test function testGenerateContent to testGenerateContent_text for clarity and consistency with other test names.
  • XCTFail Message: Enhance the XCTFail messages in the test functions to provide more context about the expected vs. actual results.

Assessment

The pull request updates the Vertex AI SDK to use the v1 API instead of v1beta. The changes include updating the base URL and API version, as well as adding new integration tests. The code appears to be well-written and the changes are straightforward. I have provided some minor feedback in the review comments. Please address these comments and have others review and approve this code before merging.

FirebaseVertexAI/Sources/Constants.swift
@@ -17,7 +17,7 @@ import Foundation
/// Constants associated with the Vertex AI for Firebase SDK.
enum Constants {
/// The Vertex AI backend endpoint URL.
static let baseURL = "https://firebasevertexai.googleapis.com"
static let baseURL = "https://staging-firebasevertexai.sandbox.googleapis.com"
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

This change updates the base URL to a staging environment. Ensure that this is intentional and that the staging environment is appropriate for the intended testing. If this is only for testing purposes, consider adding a comment explaining the use of the staging URL.

FirebaseVertexAI/Sources/GenerativeAIRequest.swift
@@ -31,7 +31,7 @@ public struct RequestOptions {
let timeout: TimeInterval

/// The API version to use in requests to the backend.
let apiVersion = "v1beta"
let apiVersion = "v1"
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

This change updates the API version to v1. Verify that all necessary API endpoints and data structures are compatible with this version.

FirebaseVertexAI/Tests/TestApp/Tests/Integration/IntegrationTests.swift

do {
let response = try await model.generateContent(fileData, "What color is this?")
XCTFail("Expected to throw an error, got response: \(response)")
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

Enhance the XCTFail message to provide more context about the expected vs. actual results. For example, include the error that was caught.

Suggested change
XCTFail("Expected to throw an error, got response: \(response)")
XCTFail("Expected to throw an error, got response: \(response), error: \(error)")

FirebaseVertexAI/Tests/TestApp/Tests/Integration/IntegrationTests.swift
_ = try await model.generateContent(prompt)
XCTFail("Expected a Firebase App Check error; none thrown.")
let response = try await model.generateContent(prompt)
XCTFail("Expected a Firebase App Check error, got response: \(response)")
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

Enhance the XCTFail message to provide more context about the expected vs. actual results. For example, include the error that was caught.

Suggested change
XCTFail("Expected a Firebase App Check error, got response: \(response)")
XCTFail("Expected a Firebase App Check error, got response: \(response), error: \(error)")

FirebaseVertexAI/Tests/TestApp/Tests/Integration/IntegrationTests.swift
for try await response in contentStream {
responses.append(response)
}
XCTFail("Expected to throw an error, got response(s): \(responses)")
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

Enhance the XCTFail message to provide more context about the expected vs. actual results. For example, include the error that was caught.

Suggested change
XCTFail("Expected to throw an error, got response(s): \(responses)")
XCTFail("Expected to throw an error, got response(s): \(responses), error: \(error)")

FirebaseVertexAI/Tests/TestApp/Tests/Integration/IntegrationTests.swift
_ = try await model.countTokens(fileData)
XCTFail("Expected to throw an error.")
let response = try await model.countTokens(fileData)
XCTFail("Expected to throw an error, got response: \(response)")
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

Enhance the XCTFail message to provide more context about the expected vs. actual results. For example, include the error that was caught.

Suggested change
XCTFail("Expected to throw an error, got response: \(response)")
XCTFail("Expected to throw an error, got response: \(response), error: \(error)")

FirebaseVertexAI/Tests/TestApp/Tests/Integration/IntegrationTests.swift
_ = try await model.countTokens(prompt)
XCTFail("Expected a Firebase App Check error; none thrown.")
let response = try await model.countTokens(prompt)
XCTFail("Expected a Firebase App Check error, got response: \(response)")
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

Enhance the XCTFail message to provide more context about the expected vs. actual results. For example, include the error that was caught.

Suggested change
XCTFail("Expected a Firebase App Check error, got response: \(response)")
XCTFail("Expected a Firebase App Check error, got response: \(response), error: \(error)")

@firebase firebase locked and limited conversation to collaborators Mar 29, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments

Assignees
No one assigned
Labels
api: vertexai
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@andrewheard