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.

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

整理: Settingdataclass へ変更 #1407

Merged
merged 2 commits into from
Jun 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
26 changes: 10 additions & 16 deletions test/unit/setting/test_setting.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
from pathlib import Path

import pytest
from pydantic import ValidationError

from voicevox_engine.setting.model import CorsPolicyMode
from voicevox_engine.setting.setting_manager import Setting, SettingHandler
from voicevox_engine.setting.setting_manager import (
Setting,
SettingHandler,
_setting_adapter,
)


def test_setting_handler_load_not_exist_file() -> None:
Expand All @@ -15,7 +16,7 @@ def test_setting_handler_load_not_exist_file() -> None:
# Expects
true_setting = {"allow_origin": None, "cors_policy_mode": CorsPolicyMode.localapps}
# Outputs
setting = settings.model_dump()
setting = _setting_adapter.dump_python(settings)
# Test
assert true_setting == setting

Expand All @@ -29,7 +30,7 @@ def test_setting_handler_load_exist_file_1() -> None:
# Expects
true_setting = {"allow_origin": None, "cors_policy_mode": CorsPolicyMode.localapps}
# Outputs
setting = settings.model_dump()
setting = _setting_adapter.dump_python(settings)
# Test
assert true_setting == setting

Expand All @@ -43,7 +44,7 @@ def test_setting_handler_load_exist_file_2() -> None:
# Expects
true_setting = {"allow_origin": None, "cors_policy_mode": "all"}
# Outputs
setting = settings.model_dump()
setting = _setting_adapter.dump_python(settings)
Comment on lines 44 to +47
Copy link
Member

Choose a reason for hiding this comment

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

true_setting側をSetting構造体にすれば_setting_adapterのimport不要にできるかもですね。
あるいはasdictするか。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍️
#1410 にて改善します。

# Test
assert true_setting == setting

Expand All @@ -60,7 +61,7 @@ def test_setting_handler_load_exist_file_3() -> None:
"cors_policy_mode": CorsPolicyMode.localapps,
}
# Outputs
setting = settings.model_dump()
setting = _setting_adapter.dump_python(settings)
# Test
assert true_setting == setting

Expand All @@ -76,13 +77,6 @@ def test_setting_handler_save(tmp_path: Path) -> None:
# Outputs
setting_loader.save(new_setting)
# NOTE: `.load()` の正常動作を前提とする
setting = setting_loader.load().model_dump()
setting = _setting_adapter.dump_python(setting_loader.load())
# Test
assert true_setting == setting


def test_setting_invalid_input() -> None:
"""`Setting` は不正な入力に対してエラーを送出する。"""
# Test
with pytest.raises(ValidationError) as _:
Setting(cors_policy_mode="invalid_value", allow_origin="*")
tarepan marked this conversation as resolved.
Show resolved Hide resolved
27 changes: 14 additions & 13 deletions voicevox_engine/setting/setting_manager.py
Original file line number Diff line number Diff line change
@@ -1,22 +1,26 @@
"""エンジン設定関連の処理"""

from dataclasses import dataclass
from enum import Enum
from pathlib import Path
from typing import Any

import yaml
from pydantic import BaseModel, Field
from pydantic import TypeAdapter

from ..utility.path_utility import get_save_dir
from .model import CorsPolicyMode


class Setting(BaseModel):
"""
エンジンの設定情報
"""
@dataclass(frozen=True)
class Setting:
"""エンジンの設定情報"""

cors_policy_mode: CorsPolicyMode = Field(title="リソース共有ポリシー")
allow_origin: str | None = Field(default=None, title="許可するオリジン")
cors_policy_mode: CorsPolicyMode # リソース共有ポリシー
allow_origin: str | None = None # 許可するオリジン


_setting_adapter = TypeAdapter(Setting)


USER_SETTING_PATH: Path = get_save_dir() / "setting.yml"
Expand All @@ -40,17 +44,14 @@ def load(self) -> Setting:
setting = {"allow_origin": None, "cors_policy_mode": "localapps"}
else:
# 指定された設定ファイルから値を取得
# FIXME: 型チェックと例外処理を追加する
# FIXME: 例外処理を追加する
setting = yaml.safe_load(self.setting_file_path.read_text(encoding="utf-8"))

return Setting(
cors_policy_mode=setting["cors_policy_mode"],
allow_origin=setting["allow_origin"],
)
return _setting_adapter.validate_python(setting)

def save(self, settings: Setting) -> None:
"""設定値をファイルへ書き込む。"""
settings_dict = settings.model_dump()
settings_dict: dict[str, Any] = _setting_adapter.dump_python(settings)
Hiroshiba marked this conversation as resolved.
Show resolved Hide resolved

if isinstance(settings_dict["cors_policy_mode"], Enum):
settings_dict["cors_policy_mode"] = settings_dict["cors_policy_mode"].value
Expand Down