feat(fancall): aioia-core Settings DI 적용 및 테스트 용이성 개선#2
Conversation
Summary of ChangesHello @ywkim, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 Pull Request는 외부 서비스 설정을 위한 의존성 주입(DI) 패턴을 도입하여 애플리케이션의 테스트 용이성을 크게 향상시키고 환경 변수 관리를 체계화하는 것을 목표로 합니다. aioia-core 라이브러리를 2.2.0으로 업데이트하고, Pydantic v2의 최신 유효성 검사 패턴을 적용하며, 애플리케이션 전용 설정 클래스를 추가하여 설정 관리의 일관성과 유지보수성을 강화했습니다. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
안녕하세요. 의존성 주입(DI) 패턴을 적용하여 테스트 용이성을 개선하고 환경 변수 관리를 체계화한 점이 인상적입니다. os.getenv를 직접 호출하는 대신 Pydantic Settings 객체를 사용하도록 리팩토링하여 코드의 명확성과 유지보수성이 크게 향상되었습니다. 특히 Pydantic v2의 model_validator를 적용하는 등 최신 모범 사례를 따르려는 노력이 돋보입니다.
한 가지 중요한 수정 사항을 제안했습니다. LiveKitSettings의 유효성 검사 로직이 의도와 다르게 동작하여 프로덕션 환경에서 개발용 기본값을 사용하는 보안 위험이 있을 수 있습니다. 제안된 코드를 참고하여 수정하시면 더욱 견고한 코드가 될 것입니다.
전반적으로 훌륭한 개선이며, 앞으로의 유지보수에 큰 도움이 될 것입니다. 수고하셨습니다!
| def check_credentials(self) -> "LiveKitSettings": | ||
| """Validate that all credentials are present together.""" | ||
| url = values.get("url") | ||
| api_key = values.get("api_key") | ||
| api_secret = values.get("api_secret") | ||
|
|
||
| # All or none should be present | ||
| provided = [url, api_key, api_secret] | ||
| provided = [self.url, self.api_key, self.api_secret] | ||
| if any(provided) and not all(provided): | ||
| raise ValueError( | ||
| "All LiveKit credentials (URL, API key, and API secret) must be provided together." | ||
| ) | ||
|
|
||
| return values | ||
| return self |
There was a problem hiding this comment.
현재 check_credentials 유효성 검사기 로직에 결함이 있어 프로덕션 환경에서 의도치 않게 개발용 기본값을 사용할 위험이 있습니다. self.url 등은 환경 변수가 설정되지 않아도 기본값을 가지므로 any(provided)와 all(provided)가 항상 True가 되어 부분적인 설정 변경을 잡아내지 못합니다. 예를 들어, LIVEKIT_URL만 프로덕션 값으로 설정하면 api_key와 api_secret은 개발용 기본값('devkey', 'secret')을 그대로 사용하게 되어 보안 문제가 발생할 수 있습니다.
Pydantic v2의 model_fields_set 속성을 사용하여 기본값이 아닌 명시적으로 설정된 필드만 확인하도록 수정해야 합니다. 이렇게 하면 일부 자격 증명만 설정되었을 때 오류를 발생시켜 완전한 설정을 강제할 수 있습니다.
| def check_credentials(self) -> "LiveKitSettings": | |
| """Validate that all credentials are present together.""" | |
| url = values.get("url") | |
| api_key = values.get("api_key") | |
| api_secret = values.get("api_secret") | |
| # All or none should be present | |
| provided = [url, api_key, api_secret] | |
| provided = [self.url, self.api_key, self.api_secret] | |
| if any(provided) and not all(provided): | |
| raise ValueError( | |
| "All LiveKit credentials (URL, API key, and API secret) must be provided together." | |
| ) | |
| return values | |
| return self | |
| def check_credentials(self) -> "LiveKitSettings": | |
| """Validate that all credentials are provided together when overriding defaults.""" | |
| credential_fields = {"url", "api_key", "api_secret"} | |
| provided_credential_fields = self.model_fields_set.intersection( | |
| credential_fields | |
| ) | |
| if 0 < len(provided_credential_fields) < len(credential_fields): | |
| raise ValueError( | |
| "All LiveKit credentials (URL, API key, and API secret) must be provided together when overriding defaults." | |
| ) | |
| return self |
There was a problem hiding this comment.
Fixed in 7ab49df. Applied model_fields_set as suggested.
외부 서비스 Settings를 aioia-core에서 import하기 위해 의존성 업데이트 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
OpenAIAPISettings, FishAudioSettings, HedraSettings를 aioia-core에서 import 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
OpenAI, Fish, Hedra Settings를 파라미터로 주입받도록 변경 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
이전 커밋에서 의도치 않게 되돌린 버전 명세 수정 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
OpenAI, Fish Audio, Hedra 설정을 환경 변수 대신 Settings 객체에서 가져오도록 변경 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
모든 Settings를 인스턴스로 생성하여 entrypoint에 전달 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Settings의 validator가 자동으로 검증하므로 중복 코드 제거 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
import 순서 및 코드 스타일 자동 수정 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
root_validator(deprecated) → model_validator로 변경하여 aioia-core 패턴과 일치 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
애플리케이션 전용 LLM 설정 (FANCALL_OPENAI_MODEL) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
FancallModelSettings를 DI로 주입받도록 변경 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
getattr 해킹 대신 FancallModelSettings.openai_model 사용 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
FancallModelSettings를 DI 체인에 추가 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Settings별로 그룹화 (LiveKit, aioia-core, Fancall, Database) - 기본값 명시 - HEDRA_*, FANCALL_OPENAI_MODEL 추가 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
model_validator 변경은 불필요했음 - Buppy와 일관성 유지 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Pydantic v2 공식 패턴 적용 (root_validator는 v3에서 제거 예정) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- worker.py에서 openai_settings 파라미터 완전히 제거 (사용되지 않음) - README 환경 변수 설명 개선 (필수 조건 명시) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- OpenAIAPISettings 유지 (일관성 + 미래 확장성) - docstring 명확화: livekit.plugins.openai가 환경변수 직접 사용 - README 환경 변수 섹션 간소화 (필수 항목만 강조) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
기본값을 가진 필드에서 validator가 작동하지 않던 문제 수정: - 기존: self.url 등은 항상 truthy하여 조건이 절대 만족 안 됨 - 변경: model_fields_set으로 명시적 설정 필드만 확인 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
7ab49df to
0181f63
Compare
There was a problem hiding this comment.
Pull request overview
이 PR은 의존성 주입(DI) 패턴을 통해 Settings 관리를 개선하고, aioia-core 2.2.0으로 업그레이드하여 테스트 용이성을 크게 향상시켰습니다. 기존 os.getenv() 직접 호출 방식에서 Settings 객체 주입 방식으로 전환하여 테스트 격리를 개선하고, Pydantic v2의 model_validator를 적용하여 현대적인 패턴을 따르도록 했습니다.
주요 변경사항:
- Settings DI 체인 구축:
main()→create_worker_options()→entrypoint()로 Settings 객체 전달 - aioia-core 2.2.0 통합:
FishAudioSettings,HedraSettings,OpenAIAPISettings재사용 - Pydantic v2 마이그레이션:
root_validator→model_validator(mode="after")및model_fields_set활용
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| backend/pyproject.toml | aioia-core 버전을 2.2.0으로 상향 조정 (^2.2.0 사용) |
| backend/poetry.lock | aioia-core 2.2.0 의존성 업데이트 및 해시 갱신 |
| backend/fancall/settings.py | FancallModelSettings 추가, LiveKitSettings validator를 Pydantic v2 패턴으로 마이그레이션 |
| backend/fancall/agent/worker.py | os.getenv() 제거, Settings DI 패턴 적용, 5개 Settings 매개변수 추가 |
| backend/README.md | 환경 변수 문서를 필수/선택으로 재구성, 기본값 관련 안내 추가 |
| ### 선택 (기능 활성화) | ||
|
|
||
| | 변수 | 기본값 | 설명 | | ||
| |------|--------|------| | ||
| | `HEDRA_ENABLED` | `false` | Hedra 아바타 활성화 | | ||
| | `HEDRA_API_KEY` | - | Hedra API 키 (enabled=true일 때 필수) | | ||
|
|
||
| > **참고**: LiveKit, 데이터베이스, 모델 등 추가 설정은 기본값으로 로컬 개발 가능합니다. | ||
| > 변경이 필요한 경우 `fancall/settings.py`의 Settings 클래스를 참고하세요. |
There was a problem hiding this comment.
새로 추가된 FANCALL_OPENAI_MODEL 환경 변수가 README의 환경 변수 섹션에 문서화되지 않았습니다.
기본값이 "gpt-4o-mini"로 설정되어 있어 선택적 변수이지만, 사용자가 다른 OpenAI 모델을 사용하고 싶을 때를 위해 문서화하는 것을 권장합니다.
"선택 (기능 활성화)" 섹션에 다음과 같이 추가할 수 있습니다:
| `FANCALL_OPENAI_MODEL` | `gpt-4o-mini` | 사용할 OpenAI LLM 모델 ||
/gemini review |
There was a problem hiding this comment.
Code Review
이 PR은 의존성 주입(DI) 패턴을 도입하여 worker.py의 테스트 용이성과 유지보수성을 크게 향상시켰습니다. os.getenv를 직접 호출하는 대신 Pydantic의 BaseSettings를 활용하여 설정을 중앙에서 관리하고 주입하는 방식은 매우 훌륭한 개선입니다. 이를 통해 설정 관련 로직이 명확해지고, 테스트 시 설정을 쉽게 모의(mock)할 수 있게 되었습니다. 또한, Pydantic v2의 model_validator를 사용하는 등 최신 모범 사례를 적용한 점도 좋습니다. 전반적으로 코드 품질을 높이는 의미 있는 변경이라고 생각합니다.
|
🎉 This PR is included in version 0.2.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
요약
aioia-core 2.2.0 외부 서비스 Settings를 활용한 의존성 주입(DI) 패턴 적용으로 테스트 용이성을 개선하고, 환경 변수 관리를 체계화했습니다.
목적
기존
os.getenv()직접 호출 방식은 테스트 시 환경 변수를 실제로 설정해야 하므로 테스트 격리가 어렵습니다. Settings 클래스 기반 DI 패턴을 적용하여 테스트에서 Settings를 mock 할 수 있도록 개선합니다.주요 개선 사항
🔧 Settings DI 패턴 적용
os.getenv("FISH_API_KEY")직접 호출fish_settings.api_keySettings 객체 주입📦 aioia-core 2.2.0 업데이트
FishAudioSettings,HedraSettingsaioia-core에서 import⚙️ FancallModelSettings 추가
FANCALL_OPENAI_MODEL)gpt-4o-mini✨ Pydantic v2 공식 패턴 적용
root_validator→model_validator(v3에서 제거 예정)LiveKitSettingsvalidator 현대화📝 README 간소화
OPENAI_API_KEY,FISH_API_KEY)구조 변경
Settings 계층
worker.py DI 체인
테스트 체크리스트
개선 효과
테스트 용이성
Before: 환경 변수 실제 설정 필요
After: Settings mock으로 격리
일관성
유지보수성
🤖 Generated with Claude Code