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

最新バージョン取得API #476

Open
3 tasks
sevenc-nanashi opened this issue Sep 20, 2022 · 17 comments
Open
3 tasks

最新バージョン取得API #476

sevenc-nanashi opened this issue Sep 20, 2022 · 17 comments
Labels
機能向上 状態:設計 設計をおこなっている状態 要議論 実行する前に議論が必要そうなもの

Comments

@sevenc-nanashi
Copy link
Member

sevenc-nanashi commented Sep 20, 2022

内容

エディタの更新情報に乗せるため、GET /update_infoのようなエンドポイントを実装した方が便利だと思いました。

Pros 良くなる点

エディタの更新情報にエンジン毎で乗せられる。

Cons 悪くなる点

(なし)

実現方法

type UpdateInfo = {
  // 新しいバージョンが利用可能かどうか。
  available: boolean;

  // バージョン。
  version?: string;

  // ダウンロード先。
  download_url?: string;
}
{
  "available": true,
  "version": "0.14.0",
  "download_url": "https://voicevox.hiroshiba.jp"
}

VOICEVOXのバージョン

0.?.0

OSの種類/ディストリ/バージョン

  • Windows
  • macOS
  • Linux

その他

(なし)

@Hiroshiba Hiroshiba added the 優先度:低 (運用中止) label Sep 23, 2022
@Hiroshiba
Copy link
Member

Hiroshiba commented Sep 23, 2022

issue作成ありがとうございます!
available_updateという名前だとboolean値が返ってきそうなので、update_infoとかがより適しているかもと思いました!

@tarepan
Copy link
Contributor

tarepan commented Feb 28, 2024

@sevenc-nanashi
こちらの機能は「エンジンが自己の次バージョン有無/アップデート可否情報を定期的にチェックし、このAPIを介して外部へ提供する」という認識で合っているでしょうか?

@sevenc-nanashi
Copy link
Member Author

「GET /update_infos を受け取ったら自己のアプデが可能かを取得してして返す」、ですね。エンジンが自動でアプデチェックとかではないです。

@tarepan
Copy link
Contributor

tarepan commented Feb 28, 2024

回答ありがとうございます!
次の流れで認識合っているでしょうか?

  1. クライアント・editor が GET /update_infos リクエストする
  2. サーバー・engine がリクエストを受け、VOICEVOX/voicevox_engine releases 等をチェックする
  3. サーバー・engine が取得情報と自身のバージョンを照合し、アプデ可否を判断する
  4. サーバー・engine がアプデ可否をレスポンスする

@sevenc-nanashi
Copy link
Member Author

ですです!
マルチエンジンでの利用を想定してる感じです。

@tarepan
Copy link
Contributor

tarepan commented Feb 28, 2024

ですです!

理解しました、ありがとうございます!

マルチエンジンでの利用を想定

👍
公式以外のマルチエンジンは「アプデしてくれー」と利用中に声を上げるすべがないため、VV API を用意するのは理にかなった設計です。
仕組みも実装も簡潔であり、良い機能になりそうと感じます。

@Hiroshiba
Copy link
Member

Hiroshiba commented Feb 28, 2024

アップデート情報ですが、エディタ側はエディターのアップデート情報を通知するようになりました!
仕組みとしてはアップデート情報を配置するURLを決めておき、ビルド時に参照される設定ファイルに書いておくとアプリ内で参照される感じです。
設定ファイルへの書き込みはこんな感じです。
https://github.com/VOICEVOX/voicevox/blob/907e2375b95ba0115b8695bb75da2d343cc45532/.env.production#L13

@tarepan tarepan added 状態:実装者募集 実装者を募集している状態 and removed 優先度:低 (運用中止) labels Mar 5, 2024
@tarepan
Copy link
Contributor

tarepan commented Apr 2, 2024

実験的実装

実験的な実装をおこないました。

get_update_info.py

"""ENGINE 更新情報の確認"""

from dataclasses import dataclass
from typing import Any
import urllib.request
import json


# Mock
@dataclass
class VersionMockHook:
    version: str
version_mock_hook = VersionMockHook("0.17.0")

def get_current_version() -> str:
    """mock"""
    return version_mock_hook.version
# /Mock


@dataclass
class UpdateInfo:
    """ENGINE 更新情報"""
    exist: bool # 起動中 ENGINE のアップデート版が存在するか否か
    version: str | None # アップデート版のバージョン(None: アップデート先無し)
    info_url: str | None # アップデート版詳細にアクセスできる URL(None: アップデート先無し)


def get_update_info() -> UpdateInfo:
    current_version = get_current_version()

    with urllib.request.urlopen("https://api.github.com/repos/VOICEVOX/voicevox_engine/releases/latest") as res:
        latest: dict[str, Any] = json.load(res)
        latest_version = latest["tag_name"]

        if current_version != latest_version:
            return UpdateInfo(exist=True, version=latest_version, info_url=latest["html_url"])
        else:
            return UpdateInfo(exist=False, version=None, info_url=None)


if __name__ == "__main__":
    version_mock_hook.version = "0.17.0"
    update_info = get_update_info()
    print(update_info)

    version_mock_hook.version = "0.18.0"
    update_info = get_update_info()
    print(update_info)

これにより、ENGINE 自体のアップデート情報を取得できます:

> python get_update_info.py
UpdateInfo(exist=True, version='0.18.0', info_url='https://github.com/VOICEVOX/voicevox_engine/releases/tag/0.18.0')
UpdateInfo(exist=False, version=None, info_url=None)

採用可否

マルチエンジンの観点から VVAPI として本機能は有用であり、上記実装例からわかるように各エンジンでの実装も容易です。
よって GET /update_info API は新設に値すると考えます。

@Hiroshiba
Go/NoGo 意見を伺えればと思います。

@tarepan tarepan added 要議論 実行する前に議論が必要そうなもの 状態:必要性議論 必要性を議論している状態 and removed 状態:実装者募集 実装者を募集している状態 labels Apr 2, 2024
@tarepan tarepan self-assigned this Apr 2, 2024
@Hiroshiba
Copy link
Member

Hiroshiba commented Apr 30, 2024

良い感じですね!!

Go/NoGo

Go寄りの気持ちです!
ちょっと考慮漏れがいくつかあったので相談させてください 🙇

将来追加したくなる情報とコンフリクトしないか

将来的に追加したくなる情報を考えておいて、その邪魔にならないか考慮しておくと安心できるかもと思いました。

考えた感じ、多分直接ダウンロードするバイナリーの URL やそのチェックsum値を配置したくなりそう。
それらはinfo_urlversionと衝突しないので大丈夫そうだなと思いました!

ちなみにelectron-builderのUdateInfoがそんな感じでした。

existキーどうするか

設計としてUpdateInfoの中にexistを持たせる以外にもいくつか方法ありそうかもです。
例えばUpdateInfoからexistをなくして、アップデートがなければNoneを返すとか。
あるいは/update_infoの返り値の型をこうするとか。

{
  exist: False,
} | {
  exist: True,
  update_info: UpdateInfo
}

このどちらかの方法であればUpdateInfo内の「アップデート先無しの場合None」を省けるかもです・・・!

@Hiroshiba
Copy link
Member

Hiroshiba commented Apr 30, 2024

ちょっと将来のことも含めて、API 実装に問題がないかという視点で考えてみました。結論は問題なさそうかなと!

  • 将来エンジンだけ自動アップデートする、とかもやりたくなったりしそう
    • その場合はエンジンのアンインストール・インストールとかをやりたくなりそう
    • そうなるとアップデート先のエンジンのバイナリーの URL が欲しくなりそう
    • もしそうなっても @tarepan さん提案の/update_info内に組み込む形で良さそう!
  • エンジンをアップデートするための API、みたいなのが欲しくなるかも
    • だとしても /update APIを生やす形にすればよさそうなので、/update_infoの設計と衝突しなさそう!
  • ライブラリやキャラクターのアップデート情報と間違うかも?
    • 仮にそういうのが欲しくなっても、prefixにlibraryとかcharacterとつけた API を作れば良さそう!

API が増えるということなので、いっぱい先のことを考えると安心できそう。
もしよければ @y-chan さんも意見いただければ!

@tarepan
Copy link
Contributor

tarepan commented Apr 30, 2024

将来的に追加したくなる情報を考えておいて、その邪魔にならないか考慮

👍️
同意です。

直接ダウンロードするバイナリーの URL やそのチェックsum値

👍️
将来拡張する際にはこんな感じになりそうです:

@dataclass
class UpdateInfo:
    """ENGINE 更新情報"""
    exist: bool # 起動中 ENGINE のアップデート版が存在するか否か
    version: str | None # アップデート版のバージョン(None: アップデート先無し)
    info_url: str | None # アップデート版詳細にアクセスできる URL(None: アップデート先無し)
    file_url: str | None # アップデート版をダウンロードできる URL(None: アップデート先無し)
    file_checksum: str | None # file_url からダウンロードされるべきファイルのチェックサム値(None: アップデート先無し)

existキーどうするか ... existをなくして、アップデートがなければNoneを返す

UpdateInfo から .exist を無くしたうえでAPI 返り値を UpdateInfo | None にする、だと理解しました。
アリだと感じます。

返り値の型をこうする

TypeScript だとこうするんですが、Python の型表現力だとこれ難しい気がします。

@dataclass
class A:
    hello: bool
    world: True

と書くと world が何故か Any に推論されます。world 値に基づいた型 narrowing も多分働かないので、おそらく Python では実益が小さいかと。

@Hiroshiba
Copy link
Member

Hiroshiba commented May 1, 2024

@tarepan

返り値の型をこうする

TypeScript だとこうするんですが、Python の型表現力だとこれ難しい気がします。

認識ずれてるかも・・・?(ずれてないかも・・・?)
こんな感じを想像してました!

@app.post("/update_info")
def update_info():
    info: UpdateInfo | None = get_info()  # 適当
    if info is None:
        return {"exist": False}
    return {"exist": True, "info": info}

(追記)とはいえUpdateInfo | Noneを返すAPIでも良さそう、どちらでも良さそうかな〜

@tarepan
Copy link
Contributor

tarepan commented May 1, 2024

API 返り値の型を以下のようにつけたいが Python 型システムの限界で実現できない、という認識でした。

class ExistUpdateInfo:
    exist: True
    info: Object

class NonExistUpdateInfo:
    exist: False

@app.post("/update_info")
def update_info() -> ExistUpdateInfo | NonExistUpdateInfo:
    info: ExistUpdateInfo | NonExistUpdateInfo = get_info()
    return info

示して頂いた感じで get_info()UpdateInfo | None を返させ router 内で ExistUpdateInfo | NonExistUpdateInfo に変換した場合、FastAPI が exist: True を正確な OpenAPI Schema に変換してくれればOKそうです(mypy は exist: Any 扱いしてくる)。

@y-chan
Copy link
Member

y-chan commented May 2, 2024

遅くなりました...!

API設計の観点からは、特にこのAPIをこのまま実装しても問題ないと思います。
この先update系のAPIが増える場合にはヒホさんの案の通りprefixで対処可能なので、気にすることもないと思います。
その他いろいろ考えてみましたが、特に懸念はなさそうです。


型についてですが、現状APIエンドポイントに関してPython側で型推論システムを使うことはおそらくないはずなので、Pythonでの恩恵はあまり考えなくても良いかもしれません。
たれぱんさんのおっしゃる通り、OpenAPI Schemaに変換された後に、エディタ側のOpenAPI Generatorが正しく型を生成し、エディタ側でexistsのtrue/falseで型推論されればエディタ側で十分な恩恵を受けられるので、その確認が出来ればこの形で実装するのが良いと思います。

@Hiroshiba
Copy link
Member

@tarepan @y-chan
僕も @y-chan さんと同じく、OpenAPIが正しく認識してくれるならそのまま押し通るのもありな気がしています。
無を返したり別のレスポンスコードを返すとなるとなんかややこしくなってくるので、個人的には直和型にしてあげたい気持ちはあります。

ただまあ・・・・OpenAPI Schemaが正しく認識してくれない予感もあります。。。
その場合は・・・204 No Contentを返しつつ、無を返す。。。かなぁ。。。

@tarepan tarepan added 状態:実装者募集 実装者を募集している状態 and removed 要議論 実行する前に議論が必要そうなもの 状態:必要性議論 必要性を議論している状態 labels May 2, 2024
@tarepan tarepan removed their assignment May 2, 2024
@sushichan044
Copy link

@y-chan の知り合いなのですが、こちらのPythonの型部分について調べてみました。

@tarepan さんのコメントではmypy上での型推論が壊れるとのことでしたが、そもそもpydanticでは以下のようにLiteral[True]Literal[False]を使用しないとランタイムエラーが発生してしまいます。

class ExistUpdateInfo(BaseModel):
    exist: Literal[True]
    info: str

class NonExistUpdateInfo(BaseModel):
    exist: Literal[False]

@app.get("/update_info", tags=["その他"])
async def update_info(arg: int) -> ExistUpdateInfo | NonExistUpdateInfo:
    if arg > 1:
        return {
            "exist": True,
            "info": "aaaa"
        }
    return { "exist": False }

また、このコードをVOICEVOXに組み込んでOpenAPI定義を出力したところ以下のようなSchemaが吐き出されており、True/Falseによる分岐自体は解釈できそうに見えました。

image

しかし、エディタのREADMEに従ってTypeScriptの型を生成して使用を試みたところ、下のような明らかに間違った型が生成されました。おそらく、OpenAPI Generatorの修正が必要かと思われます。

image

以上失礼しました。

@Hiroshiba
Copy link
Member

Hiroshiba commented May 2, 2024

@sushi-chaaaan 詳しくありがとうございます!!
なるほど、今のとこまだLiteralで型付けないとダメだったんですね。。
Python 3.12とかだともしかしたら動いてたかもですね!

generator側は失念していました! 結論としては、まあ直和型は避けたほうが良さそうですね!!!
(generator側修正はとても大変なので・・・)

@tarepan tarepan added 要議論 実行する前に議論が必要そうなもの 状態:設計 設計をおこなっている状態 and removed 状態:実装者募集 実装者を募集している状態 labels May 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
機能向上 状態:設計 設計をおこなっている状態 要議論 実行する前に議論が必要そうなもの
Projects
None yet
Development

No branches or pull requests

5 participants