ArkMowers / arknights-mower

《明日方舟》长草助手
https://arkmowers.github.io/arknights-mower/
MIT License
505 stars 53 forks source link

【Refac Suggestion】 使用 Pydantic 重构 `Config` #648

Closed MuelNova closed 1 month ago

MuelNova commented 1 month ago

TL; DR

利用 Pydantic 重构当前仅为一个 dict 的 conf 对象,使其具备可维护性,并实现数据验证与类型注解等功能。

Why

Situation

目前的 conf 对象极其缺乏可维护性,对于键值的处理极其容易出现 KeyError 的情况。

        send_message_config = getattr(self, "send_message_config", None)
        if not send_message_config:
            logger.error("send_message_config 未在配置中定义!")
            return

        failed_methods = []

        if subtype == "plain" and subject == "":
            subject = body

        # markdown格式的消息体
        mkBody = body
        # 如果body是HTML内容,转换为Markdown格式
        if subtype == "html":
            mkBody = self.html_to_markdown(body)

        # 获取QQ邮件配置
        email_config = send_message_config.get("email_config")
        # 获取Server酱配置
        serverJang_push_config = send_message_config.get("serverJang_push_config")

以代码为例,在开发过程中,send_message_configemail_config 等均为 Any 类型,对于开发造成极大的困难,极其容易出现 Typo (例如在一个地方设置配置 conf['foo'] = 'bar',而调用时使用 conf.get('fo0', 'bee'),且没有类型注解也对后续开发以及项目可维护性影响较大。具体来说,删除一个配置项的代价是极为高昂的,因为我们不能很快了解任意配置项在哪些地方被引用,也不会在 lint 阶段由编辑器标出错误,指明哪些字段是不存在的。

Inspiration

Pydantic 是一个广泛利用的数据验证库,并且对 IDE 和 Linter 有很好的兼容,利用 Pydantic,我们可以获得类型注解,并且在类型不匹配的情况下给出运行时错误 ValidationError 以告知开发者。下面是一个 Pydantic 正确运行的例子。

from datetime import datetime

from pydantic import BaseModel, PositiveInt

class User(BaseModel):
    id: int  
    name: str = 'John Doe'  
    signup_ts: datetime | None  
    tastes: dict[str, PositiveInt]  

external_data = {
    'id': 123,
    'signup_ts': '2019-06-01 12:22',  
    'tastes': {
        'wine': 9,
        b'cheese': 7,  
        'cabbage': '1',  
    },
}

user = User(**external_data)  

print(user.id)  
#> 123
print(user.model_dump())  
"""
{
    'id': 123,
    'name': 'John Doe',
    'signup_ts': datetime.datetime(2019, 6, 1, 12, 22),
    'tastes': {'wine': 9, 'cheese': 7, 'cabbage': 1},
}
"""

在错误情况下,我们也可以获得更多信息。

# continuing the above example...

from pydantic import ValidationError

class User(BaseModel):
    id: int
    name: str = 'John Doe'
    signup_ts: datetime | None
    tastes: dict[str, PositiveInt]

external_data = {'id': 'not an int', 'tastes': {}}  

try:
    User(**external_data)  
except ValidationError as e:
    print(e.errors())
    """
    [
        {
            'type': 'int_parsing',
            'loc': ('id',),
            'msg': 'Input should be a valid integer, unable to parse string as an integer',
            'input': 'not an int',
            'url': 'https://errors.pydantic.dev/2/v/int_parsing',
        },
        {
            'type': 'missing',
            'loc': ('signup_ts',),
            'msg': 'Field required',
            'input': {'id': 'not an int', 'tastes': {}},
            'url': 'https://errors.pydantic.dev/2/v/missing',
        },
    ]
    """

利用 Pydantic,我们可以更加清晰地配置配置项,并且不再需要利用 temp_conf 等低效的 io 密集型操作进行配置的初始化,可以显著减小 /conf 路由的开销。同时,得益于 Pydantic 的高自定义性,我们也可以对旧 Config 进行较为迅速的迁移。

How

由于 Arknights Mower 配置项较为复杂,同时将所有配置项写在一个 BaseModel 对象显然也不利于可维护性。可以将配置项进行分类然后在大的 Config 对象里进行组装。

在这个情况下,我们几乎需要对所有使用 conf 的地方进行重写,工作量无疑是巨大的。在思想实验中,我总结出了以下几个困难:

  1. 当前 config 覆盖较广,修改较为困难。
  2. 当前 config 存在直接添加字段的情况,这对于 Pydantic Model 是不可接受的,因此需要找出所有字段并且对相关代码进行修改。
  3. 修改 config 后的代码正确性难以测试(似乎目前没有单测相关的代码?)。
  4. Pydantic 对不熟悉 python type hint 以及不熟悉强类型语言的开发者较为不友好,其学习成本较高。在某些意义上,它违背了 Python 弱类型的特点。

作为该 Suggestion 的提出者,我可以承担主要开发工作。然而,由于个人科研任务,可能进度缓慢,因此,首先提出这个 Suggestion Issue,等待各位大佬的讨论、建议及意见。

MuelNova commented 1 month ago

A quick demo

测试代码

class Config(BaseModel):
    adb: str = Field("127.0.0.1:62001", description="ADB 连接地址")
    drone_count_limit: int = Field(92, description="无人机使用阈值", ge=0, le=120)

    __EXCLUDE__ = {"adb_host", "adb_port"}

    ...  # other configurations

    @computed_field
    @property
    def adb_host(self) -> str:
        """
        ADB 主机地址
        """
        return self.adb.split(":")[0]

    @computed_field
    @property
    def adb_port(self) -> int:
        """
        ADB 端口号
        """
        return int(self.adb.split(":")[-1])

    @field_validator("adb", mode="after")
    @classmethod
    def validate_adb(cls, value: str) -> str:
        port = int(value.split(":")[-1])
        if port < 1 or port > 65535:
            raise ValueError("ADB 端口号范围应为 1-65535")
        return value

    @staticmethod
    def load_conf(file: Path | str = Path("./conf2.yml")) -> "Config":
        with open(file, "r", encoding="utf8") as f:
            return Config.model_validate(yaml.load(f, Loader=CoreLoader))

    def save_conf(self, file: Path | str = Path("./conf2.yml")) -> None:
        with open(file, "w", encoding="utf8") as f:
            yaml.dump(
                self.model_dump(exclude=self.__EXCLUDE__),
                f,
                Dumper=CoreDumper,
                encoding="utf-8",
                default_flow_style=False,
                allow_unicode=True,
            )
from pydantic import ValidationError

from foobar.model import Config

# Default
config = Config()
print(config)
print(config.adb_port)
print(config.adb_host)

# Invalid
try:
    err_config = Config(adb="127.0.0.1:66666", drone_count_limit=121)
    print(err_config)
except ValidationError as e:
    print(e)

# Serialize
config.save_conf("test_conf.yml")

输出:

❯ python test.py
adb='127.0.0.1:62001' drone_count_limit=92 adb_host='127.0.0.1' adb_port=62001
62001
127.0.0.1
2 validation errors for Config
adb
  Value error, ADB 端口号范围应为 1-65535 [type=value_error, input_value='127.0.0.1:66666', input_type=str]
    For further information visit https://errors.pydantic.dev/2.8/v/value_error
drone_count_limit
  Input should be less than or equal to 120 [type=less_than_equal, input_value=121, input_type=int]
    For further information visit https://errors.pydantic.dev/2.8/v/less_than_equal

❯ cat test_conf.yml
adb: 127.0.0.1:62001
drone_count_limit: 92
MuelNova commented 1 month ago

这个 conf 比我想的还要屎,plan 和 conf 还有一堆乱七八糟的都写在一起,充分体现了其不可维护性 (:IJ<)

ZhaoZuohong commented 1 month ago
class Config(BaseModel):
    adb: str = Field("127.0.0.1:62001", description="ADB 连接地址")
    drone_count_limit: int = Field(92, description="无人机使用阈值", ge=0, le=120)

    __EXCLUDE__ = {"adb_host", "adb_port"}

    ...  # other configurations

    @computed_field
    @property
    def adb_host(self) -> str:
        """
        ADB 主机地址
        """
        return self.adb.split(":")[0]

    @computed_field
    @property
    def adb_port(self) -> int:
        """
        ADB 端口号
        """
        return int(self.adb.split(":")[-1])

    @field_validator("adb", mode="after")
    @classmethod
    def validate_adb(cls, value: str) -> str:
        port = int(value.split(":")[-1])
        if port < 1 or port > 65535:
            raise ValueError("ADB 端口号范围应为 1-65535")
        return value

没有考虑 emulator-5554c2b1c2a7 的情况。

ZhaoZuohong commented 1 month ago

如果只是以“保持现有配置文件的格式不变,添加检查和类型标注”为目的,那我觉得非常不值。

MuelNova commented 1 month ago

如果只是以“保持现有配置文件的格式不变,添加检查和类型标注”为目的,那我觉得非常不值。

是的,所以目前的考虑是先做迁移,再做分离 / 合并调整。

考虑到现在的配置项已经有 300 多行,一些配置项已经弃用/搬家(例如 plan 相关的几个),而另一些比较臃肿(例如周计划,现在的写法感觉不是很好?),长期继续使用下去对于代码正确性以及代码可维护性的保证都会降低。值不值这个确实,能跑就没必要改,不过我觉得会不会因为这样劝退一些人

目前我的一个简单的分离措施是把配置项按照首页前端的分块进行简单的划分,归到不同的子模块下,不知道有没有更好的方法?

MuelNova commented 1 month ago
class Config(BaseModel):
    adb: str = Field("127.0.0.1:62001", description="ADB 连接地址")
    drone_count_limit: int = Field(92, description="无人机使用阈值", ge=0, le=120)

    __EXCLUDE__ = {"adb_host", "adb_port"}

    ...  # other configurations

    @computed_field
    @property
    def adb_host(self) -> str:
        """
        ADB 主机地址
        """
        return self.adb.split(":")[0]

    @computed_field
    @property
    def adb_port(self) -> int:
        """
        ADB 端口号
        """
        return int(self.adb.split(":")[-1])

    @field_validator("adb", mode="after")
    @classmethod
    def validate_adb(cls, value: str) -> str:
        port = int(value.split(":")[-1])
        if port < 1 or port > 65535:
            raise ValueError("ADB 端口号范围应为 1-65535")
        return value

没有考虑 emulator-5554c2b1c2a7 的情况。

是的,我原意是展示一下 Pydantic 对于数据正确性的保证以及它强大的自定义功能,以及对于错误的 handle 能力,但是在编写过程中就意识到举的例子并不恰当。

同时我也注意到前端是直接 watch,在修改过程中持续 post 后端的,因此在修改配置的过程中,如果中间输入不合法,就会一直产生 validation error,所以可能还不太适用我一开始考虑的将 validation error 内容发到前端提示的想法(一个想法是输入框失去焦点再 post?)

ZhaoZuohong commented 1 month ago

一些配置项已经弃用/搬家(例如 plan 相关的几个)

目前的配置文件里没有 plan 相关的,只有一个 planFile

比较臃肿(例如周计划

周计划就一个 dict 扔在那里先不管吧。

产生 validation error

我觉得验证暂时没必要加。

把配置项按照首页前端的分块进行简单的划分,归到不同的子模块下

我的想法是,现有配置文件的格式保持不变,修改后让每一项设置都有:(1)默认值、(2)说明、(3)类型:

比如原来的一部分配置是这样的:

account: zhbaor@zhaozuohong.vip
adb: localhost:5555
check_mail_enable: true
close_simulator_when_idle: false
credit_fight:
  direction: Right
  operator: 风笛
  squad: 1
  x: 5
  y: 3
custom_screenshot:
  command: adb -s 127.0.0.1:5555 shell screencap -p 2>/dev/null
  enable: false

直接变成

from pydantic import BaseModel, model_validator

class ConfModel(BaseModel):
    @model_validator(mode="before")
    @classmethod
    def nested_defaults(cls, data):
        for name, field in cls.model_fields.items():
            if issubclass(field.annotation, BaseModel) and name not in data:
                data[name] = field.annotation()
        return data

class CreditFightConf(ConfModel):
    direction: str = "Right"
    "部署方向"
    operator: str = "风笛"
    "使用干员"
    squad: int = 1
    "编队序号"
    x: int = 5
    "横坐标"
    y: int = 3
    "纵坐标"

class CustomScreenshotConf(ConfModel):
    command: str = "adb -s 127.0.0.1:5555 shell screencap -p 2>/dev/null"
    "截图命令"
    enable: bool = False
    "是否启用自定义截图"

class Conf(ConfModel):
    account: str = ""
    "邮箱用户名"
    adb: str = "127.0.0.1:16384"
    "ADB连接地址"
    check_mail_enable: bool = True
    "是否领取邮件奖励"
    close_simulator_when_idle: bool = False
    "任务结束后关闭游戏"
    credit_fight: CreditFightConf
    "信用作战相关设置"
    custom_screenshot: CustomScreenshotConf
    "自定义截图相关配置"
MuelNova commented 1 month ago

关于最后一个,可以看 https://github.com/ArkMowers/arknights-mower/pull/649/commits/9282f00cc434303991eff1efba1456c95de9afbc#diff-fb17e63277f26ae663ecde3e2215b7d95a1781e3f8665a0dde721474943c6661R23

这是目前的,并且兼容了原本的使用方法。

ZhaoZuohong commented 1 month ago

可以看 https://github.com/ArkMowers/arknights-mower/commit/9282f00cc434303991eff1efba1456c95de9afbc#diff-fb17e63277f26ae663ecde3e2215b7d95a1781e3f8665a0dde721474943c6661R23

我不喜欢 xxx : SomeType = Field("default value", default_factory=SomeType, description="description") 的写法,很长,有default_factory 时类型需要写两遍,而且 IDE 无法提示 description。

class Conf(ConfModel):
    account: str = ""
    "邮箱用户名"
    adb: str = Field("127.0.0.1:62001", description="ADB 连接地址")

c = Conf()

c.account
c.adb

image image