-
Notifications
You must be signed in to change notification settings - Fork 23
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.
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
base: main
Are you sure you want to change the base?
feat: add Google Vertex AI connector implementation #493
Conversation
…fra bicep templates
…rtexAIConnectorTests
오셨군요! 내용 확인해보겠습니다 ~ |
리뷰 진행상황
리뷰 백로그 / 기타 메모
|
|
There was a problem hiding this comment.
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 수정이 필요합니다
피드백 주신 부분 하나씩 수정하면서 코멘트 남기겠습니다! |
우선 PR 본문 정리했습니다! |
|
[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) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Type expectedType
매개인자는 사용하지 않으니 모두 제거해주세요
다른 테스트 메서드도 모두 동일합니다 ~
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
10/10 수정요청입니다 LanguageModelConnector 상속 테스트는 잘 확인했습니다!
- 테스트 메서드에서 불필요한 매개인자 제거하기
- 지난번 요청한 문서작업
불필요한 매개인자 제거했습니다! |
@hxcva1 넵 확인했습니다! |
|
||
1. Get the repository root. | ||
|
||
```bash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드블럭은 한 탭 들여 씁니다
그래야 1. 만 써도 1, 2, 3으로 순번이 잘 넘어갈 거에요
아래 github models 문서를 참고해주세요
https://github.com/aliencube/open-chat-playground/blob/main/docs/github-models.md?plain=1
set GoogleVertexAI:ApiKey "{{GOOGLE_VERTEX_AI_API_KEY}}" | ||
``` | ||
|
||
> Note: code and tests in this repository use the config keys `GoogleVertexAI:ApiKey` and `GoogleVertexAI:Model`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이런 각주도 잊지 말고 한 탭 들여 써주세요.
azd down --force --purge | ||
``` | ||
|
||
## Notes & troubleshooting |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 아래 내용은 모두 삭제하고, 필요하면 이 PR 본문 혹은 이슈에 코멘트로 남겨주세요.
cd $REPOSITORY_ROOT | ||
``` | ||
|
||
1. Add Google Vertex AI API Key. Replace `{{GOOGLE_VERTEX_AI_API_KEY}}` with your key. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
API KEY 발급방법을 전부 다 설명할 필요는 없지만
조금만 더 친절하게 어떤 구글 공식문서를 참고하면 되는지만 링크를 달아주세요
이런식으로요
> For more details about GitHub PAT, refer to the doc, [Managing your personal access tokens](https://docs.github.com/authentication/keeping-your-account-and-data-secure/managing-your-personal-access-tokens). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생많으셨습니다! ㅎㅎ
문서 스타일을 맞추고, API KEY 발급시 참고할 문서 링크를 추가해주세요.
@hxcva1 잘 지내시죠? 진행상황 업데이트 부탁드립니다 |
그러면 저는 우선 테스트 코드 보완 부터 시도해보도록 하겠습니다 |
Github 상에서 Conflict 해결하고 커밋을 했는데 로컬에서 이 버전을 pull 안하고 작업한 내용을 커밋했는데 푸쉬하려고 하니 에러가 나서 어떻게 해야할까요? rebase 기능을 사용해도 되는지 git이 아직 완전히 익숙하지는 않아서 죄송합니다. |
git pull 해보셨나요? conflict 발생하면 로컬에서 수정하고 commit & push 하시면 됩니다. |
Purpose
Does this introduce a breaking change?
Pull Request Type
What kind of change does this Pull Request introduce?
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?
What to Check
Verify that the following are valid