-
Notifications
You must be signed in to change notification settings - Fork 118
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
C API用のzipファイルからmodelディレクトリをなくし、別でmodelダウンロード可能にする #603
The head ref may contain hidden characters: "\u30B3\u30A2\u306Emodel\u30C7\u30A3\u30EC\u30AF\u30C8\u30EA\u3092\u5206\u96E2"
Conversation
ダウンローダーを変更するとダウンローダーのテストが実行されるのですが、releasesのlatestのファイルを見に行くのでmodel.zipが存在せずテストが落ちてしまいそうです。 でもテストが多すぎる問題を解決したプルリクエストでワークフローを分けた関係で、リリース後にダウンロードのテストをするのが難しくなっています(2つのワークフローが完了した後に実行しないといけない)。 解決策としては、 |
crates/download/src/main.rs
Outdated
let model = find_gh_asset(octocrab, CORE_REPO_NAME, &version, |tag| { | ||
format!("model-{tag}.zip") | ||
}) | ||
.await?; |
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.
find_gh_asset
をfind_gh_assets
にすればアクセス回数を一つ減らせそうではありますね。以下のようにまですべきかどうかはともかくとして。
- let core = find_gh_asset(octocrab, CORE_REPO_NAME, &version, |tag| {
+ let [core, model] = find_gh_assets(octocrab, CORE_REPO_NAME, &version, |tag| {
let device = match (os, device) {
(Os::Linux, Device::Cuda) => "gpu",
(_, device) => device.into(),
};
- format!("{CORE_REPO_NAME}-{os}-{cpu_arch}-{device}-{tag}.zip")
- })
- .await?;
-
- let model = find_gh_asset(octocrab, CORE_REPO_NAME, &version, |tag| {
- format!("model-{tag}.zip")
+ [
+ format!("{CORE_REPO_NAME}-{os}-{cpu_arch}-{device}-{tag}.zip"),
+ format!("model-{tag}.zip"),
+ ]
})
.await?;
let additional_libraries = OptionFuture::from((device != Device::Cpu).then(|| {
- find_gh_asset(
+ find_gh_assets(
octocrab,
ADDITIONAL_LIBRARIES_REPO_NAME,
&additional_libraries_version,
|_| {
let device = match device {
Device::Cpu => unreachable!(),
Device::Cuda => "CUDA",
Device::Directml => "DirectML",
};
- format!("{device}-{os}-{cpu_arch}.zip")
+ [format!("{device}-{os}-{cpu_arch}.zip")]
},
)
}))
.await
- .transpose()?;
+ .transpose()?
+ .map(|[asset]| asset);
-async fn find_gh_asset(
+async fn find_gh_assets<const N: usize>(
octocrab: &Arc<Octocrab>,
repo: &str,
git_tag_or_latest: &str,
- asset_name: impl FnOnce(&str) -> String,
-) -> anyhow::Result<GhAsset> {
+ asset_names: impl FnOnce(&str) -> [String; N],
+) -> anyhow::Result<[GhAsset; N]> {
let Release {
html_url,
tag_name,
assets,
..
} = {
let repos = octocrab.repos(ORGANIZATION_NAME, repo);
let releases = repos.releases();
match git_tag_or_latest {
"latest" => releases.get_latest().await,
tag => releases.get_by_tag(tag).await,
}?
};
- let asset_name = asset_name(&tag_name);
- let Asset { id, name, size, .. } = assets
- .into_iter()
- .find(|Asset { name, .. }| *name == asset_name)
- .with_context(|| format!("Could not find {asset_name:?} in {html_url}"))?;
+ return asset_names(&tag_name).try_map_(|asset_name| {
+ let Asset { id, name, size, .. } = assets
+ .iter()
+ .find(|Asset { name, .. }| *name == asset_name)
+ .with_context(|| format!("Could not find {asset_name:?} in {html_url}"))?;
+
+ Ok(GhAsset {
+ octocrab: octocrab.clone(),
+ repo: repo.to_owned(),
+ tag: tag_name.clone(),
+ id: *id,
+ name: name.clone(),
+ size: *size as _,
+ })
+ });
- Ok(GhAsset {
- octocrab: octocrab.clone(),
- repo: repo.to_owned(),
- tag: tag_name,
- id,
- name,
- size: size as _,
- })
+ #[easy_ext::ext]
+ impl<T, const N: usize> [T; N] {
+ fn try_map_<F, O, E>(self, f: F) -> Result<[O; N], E>
+ where
+ F: FnMut(T) -> Result<O, E>,
+ {
+ let vec = self.into_iter().map(f).collect::<Result<Vec<_>, _>>()?;
+ Ok(vec
+ .try_into()
+ .unwrap_or_else(|_| unreachable!("the lengths should be same")))
+ }
+ }
}
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.
すみません、まとめた方が良さそうというのはわかるのですが、ちょっと難しすぎそうなのでパスさせてください 🙇
レビューありがとうございます!!ドラフト状態なので他のプルリクエストが完了次第反映できればと思います! |
e5f5ade
to
2e93f10
Compare
ビルドが通ったのでドラフト開けました! |
ビルドワークフローの実行結果です。
下がダウンローダー、上がライブラリのビルドです。 |
@@ -255,7 +267,7 @@ fn octocrab() -> octocrab::Result<Arc<Octocrab>> { | |||
|
|||
async fn find_gh_asset( | |||
octocrab: &Arc<Octocrab>, | |||
repo: RepoName, | |||
repo: &RepoName, |
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.
ここをrepo: RepoName
にしたのはC-CALLER-CONTROLに従った結果だったのですが、まあどちらでも良いと思います。
こういう考え方も主流ですし(というか私も普段書き殴るときはどちらかというとこっち側です)
https://twitter.com/qnighy/status/1008010536397651968
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.
あんま深いこと考えてなくて、find_gh_asset
を2回呼ぶのでrepo
が移譲?されちゃって以降で使えなくなっちゃったんで&
つけた感じでした!
Co-authored-by: Ryo Yamashita <[email protected]>
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.
LGTM!
ビルドワークフローが通りました!!!!! ありがとうございました!!!! リリースこちら |
0.15.0-preview.12 ビルドを開始しました! |
内容
タイトルの通り、C API用のzipファイルからmodelディレクトリをなくし、別でmodelダウンロード可能にします。
ビルド時にFAT RESOURCEやサンプルのmodelディレクトリを導入させず、別でそのディレクトリのZIPファイルを作成してアップロードしています。
ダウンロードでは
min
を指定していない場合に辞書ファイルなどと一緒にモデルディレクトリがダウンロードされる感じです。関連 Issue
fix #596
その他
サンプルでのビルド結果こちらです。
https://github.com/Hiroshiba/voicevox_core/releases/tag/0.15.0-checkmodelsplit.16
の変更が含まれているので先にそちらをマージする必要があるかもです。