Skip to content

feat: add Google Vertex AI connector implementation #493

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
Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

feat: add Google Vertex AI connector implementation #493

wants to merge 12 commits into from
+271 −1

Conversation

hxcva1
Copy link
Contributor

@hxcva1 hxcva1 commented Oct 8, 2025 •
edited
Loading

Purpose

Does this introduce a breaking change?

[ ] Yes
[x] No

Pull Request Type

What kind of change does this Pull Request introduce?

[ ] Bugfix
[x] New feature
[ ] Refactoring (no functional changes, no api changes)
[ ] Documentation content changes
[ ] Other... Please describe:

README updated?

The top-level readme for this repo contains a link to each sample in the repo. If you're adding a new sample did you update the readme?

[x] Yes
[ ] No
[ ] N/A

What to Check

Verify that the following are valid

  • Required Parameters: ApiKey, Model

hxcva1 added 10 commits September 16, 2025 19:52
@hxcva1
feat: Add GoogleVertexAIConnector with settings validation and stubbe…
448aa7f 
…d chat client
@hxcva1
feat: Add GoogleVertexAIConnector to LanguageModelConnector factory
389481a
@hxcva1
feat: Add Google Vertex AI parameters and environment variables to in…
12d2bd6 
…fra bicep templates
@hxcva1
feat: Implement GoogleVertexAIConnector using GeminiChatClient
0be2a59
@hxcva1
test: Add unit tests for GoogleVertexAIConnector
4c28c4c
@hxcva1
test: Fix failing tests for GoogleVertexAIConnector when model is mis…
319f6be 
…sing
@hxcva1
chore: Resolve conflicts between main.bicep and main.parameters.json
4d05176
@hxcva1
refactor: Replace magic strings with private const string in GoogleVe…
2dff61a 
…rtexAIConnectorTests
@hxcva1
Merge branch 'main' into feature/237-connector-impl-google-vertex-ai
274a22a
@hxcva1
test: Add new test cases to GoogleVertexAIConnectorTests
d2e7f6d
@justinyoo justinyoo linked an issue Oct 8, 2025 that may be closed by this pull request
Connector Implementation & Inheritance: Google Vertex AI #237
Open
@justinyoo justinyoo requested a review from tae0y October 8, 2025 16:00
@tae0y
Copy link
Member

tae0y commented Oct 9, 2025

오셨군요! 내용 확인해보겠습니다 ~

@tae0y
Copy link
Member

tae0y commented Oct 9, 2025 •
edited
Loading

리뷰 진행상황

  • replacement of feat: add Google Vertex AI connector implementation #454
  • 지난 PR에서 논의/확인했던 사항
    • GeminiChatClient 사용해서 Connector 구현
    • 테스트 시나리오 커버리지
      • GoogleVertexAIConnector가 LanguageModelConnector를 상속하는지에 대한 테스트
      • Connector 로직 테스트
        • EnsureLanguageModelSettingsValid()
        • GetChatClientAsync()
        • LanguageModelConnector.CreateChatClientAsync()
        • 예외발생 테스트 컨벤션 Tests are NOT testing exceptions #451
        • 하드코딩된 magic string 상수화
      • 경계값 테스트
      • LanguageModelConnector.cs case문에서 LLM Provider 지원순서를 지키기

리뷰 백로그 / 기타 메모

  • 그래서 이 커넥터는 잘 동작하는가 ...
  • 문서 : 호스트머신, 컨테이너 부분
  • Bicep 구현
  • 문서 : Bicep/Azure 부분

@tae0y
Copy link
Member

tae0y commented Oct 9, 2025

test/OpenChat.PlaygroundApp.Tests/Connectors/GoogleVertexAIConnectorTests.cs 테스트 시나리오 정리

  • Given_Settings_Is_Null_When_EnsureLanguageModelSettingsValid_Invoked_Then_It_Should_Thro
    • GoogleVertexAI 설정이 null일 때 EnsureLanguageModelSettingsValid 호출 시 예외가 발생해야 함
  • Given_Invalid_ApiKey_When_EnsureLanguageModelSettingsValid_Invoked_Then_It_Should_Thro
    • ApiKey가 null, 빈 문자열, 공백만 있을 때 EnsureLanguageModelSettingsValid 호출 시 예외가 발생해야 함
  • Given_Invalid_Model_When_EnsureLanguageModelSettingsValid_Invoked_Then_It_Should_Thro
    • Model이 null, 빈 문자열, 공백만 있을 때 EnsureLanguageModelSettingsValid 호출 시 예외가 발생해야 함
  • Given_Valid_Settings_When_EnsureLanguageModelSettingsValid_Invoked_Then_It_Should_Return_Tru
    • 올바른 설정이 주어졌을 때 EnsureLanguageModelSettingsValid 호출 시 true를 반환해야 함
  • Given_Valid_Settings_When_GetChatClient_Invoked_Then_It_Should_Return_ChatClien
    • 올바른 설정이 주어졌을 때 GetChatClientAsync 호출 시 ChatClient가 반환되어야 함
  • Given_Settings_Is_Null_When_GetChatClientAsync_Invoked_Then_It_Should_Thro
    • GoogleVertexAI 설정이 null일 때 GetChatClientAsync 호출 시 예외가 발생해야 함
  • Given_Missing_ApiKey_When_GetChatClient_Invoked_Then_It_Should_Thro
    • ApiKey가 null 또는 빈 문자열일 때 GetChatClientAsync 호출 시 예외가 발생해야 함
  • Given_Missing_Model_When_GetChatClient_Invoked_Then_It_Should_Thro
    • Model이 null일 때 GetChatClientAsync 호출 시 예외가 발생해야 함
  • Given_Valid_Settings_When_CreateChatClientAsync_Invoked_Then_It_Should_Return_ChatClien
    • 올바른 설정이 주어졌을 때 LanguageModelConnector.CreateChatClientAsync 호출 시 IChatClient가 반환되어야 함
  • Given_Invalid_Settings_When_CreateChatClientAsync_Invoked_Then_It_Should_Thro
    • 잘못된 설정(예: ApiKey가 null)이 주어졌을 때 LanguageModelConnector.CreateChatClientAsync 호출 시 예외가 발생해야 함

tae0y
tae0y requested changes Oct 9, 2025
View reviewed changes
Copy link
Member

@tae0y tae0y left a comment •
edited
Loading

Choose a reason for hiding this comment

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

10/9 변경 요청사항

  • 아래 LanguageModelConnectorTests에서 GoogleVertexAIConnector 구문 각주해제
    • test/OpenChat.PlaygroundApp.Tests/Abstractions/LanguageModelConnectorTests.cs
    • GoogleVertexAIConnector가 LanguageModelConnector를 잘 상속하는지 테스트
    • 관련내용은 이 이슈 참조 #423

그리고 Connector로직과 테스트가 어느정도 정리되었으니 문서로 넘어가시죠!

  • PR본문 정리 : 불필요한 부분은 남겨두지 마시고 삭제해주세요
  • docs 디렉토리 아래에 googlevertexai 문서화해주세요
    • docs/openai, docs/huggingface를 참고해서 작성하시면 됩니다
    • 다른 분들 PR에서 자주 언급됐던 부분을 공유드리면
      • bash/powershell로 구분하여 샘플을 작성해주세요
      • bash 명령어에서 강제개행은 / 역슬래시, powershell은 ` 백틱을 사용합니다
      • docker 컨테이너를 말 때 Github container registry에서 이미지 가져오는 샘플도 작성해주세요
  • 그밖에 리포지토리 루트, docs 루트에 있는 README 수정이 필요합니다

@hxcva1
Copy link
Contributor Author

hxcva1 commented Oct 9, 2025

피드백 주신 부분 하나씩 수정하면서 코멘트 남기겠습니다!

@hxcva1
Copy link
Contributor Author

hxcva1 commented Oct 10, 2025

우선 PR 본문 정리했습니다!

@hxcva1
Copy link
Contributor Author

hxcva1 commented Oct 10, 2025

test/OpenChat.PlaygroundApp.Tests/Abstractions/LanguageModelConnectorTests.cs 부분 주석 해제 했습니다!

tae0y
tae0y reviewed Oct 10, 2025
View reviewed changes
test/OpenChat.PlaygroundApp.Tests/Connectors/GoogleVertexAIConnectorTests.cs Outdated
[InlineData(null, typeof(InvalidOperationException), "GoogleVertexAI:ApiKey")]
[InlineData("", typeof(InvalidOperationException), "GoogleVertexAI:ApiKey")]
[InlineData(" ", typeof(InvalidOperationException), "GoogleVertexAI:ApiKey")]
public void Given_Invalid_ApiKey_When_EnsureLanguageModelSettingsValid_Invoked_Then_It_Should_Throw(string? apiKey, Type expectedType, string expectedMessage)
Copy link
Member

Choose a reason for hiding this comment

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

Type expectedType 매개인자는 사용하지 않으니 모두 제거해주세요
다른 테스트 메서드도 모두 동일합니다 ~

tae0y
tae0y requested changes Oct 10, 2025
View reviewed changes
Copy link
Member

@tae0y tae0y left a comment •
edited
Loading

Choose a reason for hiding this comment

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

10/10 수정요청입니다 LanguageModelConnector 상속 테스트는 잘 확인했습니다!

  • 테스트 메서드에서 불필요한 매개인자 제거하기
  • 지난번 요청한 문서작업

@hxcva1
Copy link
Contributor Author

hxcva1 commented Oct 13, 2025

불필요한 매개인자 제거했습니다!

@tae0y
Copy link
Member

tae0y commented Oct 13, 2025

@hxcva1 넵 확인했습니다!
문서 작업으로 넘어가시죵

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tae0y tae0y tae0y requested changes

Assignees

@hxcva1 hxcva1

Labels

maas ossca p1 phase-1 task team-park

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Connector Implementation & Inheritance: Google Vertex AI

3 participants

@hxcva1 @tae0y @justinyoo