refactor: エンジン情報マネージャーとエンジンプロセスマネージャーを分ける #2260
Conversation
| await engineManager.restartEngine(engineId); | ||
| // TODO: setEngineInfosからexportFileはロックしたほうがより良い | ||
| runtimeInfoManager.setEngineInfos(engineManager.fetchEngineInfos()); | ||
| await engineProcessManager.restartEngine(engineId); | ||
|
|
||
| // ランタイム情報の更新 | ||
| // TODO: setからexportの処理は排他処理にしたほうがより良い | ||
| runtimeInfoManager.setEngineInfos(engineInfoManager.fetchEngineInfos()); |
There was a problem hiding this comment.
処理は変わってないです。
「ロックしたほうがより良い」の意味が分かりづらかったので、ロック→排他処理に置き換えました。
There was a problem hiding this comment.
このファイルは以前のEngineManagerからのコード移動だけになってるはず。
There was a problem hiding this comment.
現状特に何も管理していないのにManagerになってたので、Helperに変更してみました。
| /** | ||
| * 代替ポート情報を更新する。 | ||
| * エンジン起動時にポートが競合して代替ポートを使う場合に使用する。 | ||
| */ | ||
| updateAltPort(engineId: EngineId, port: number) { | ||
| const engineInfo = this.fetchEngineInfo(engineId); | ||
| const url = new URL(engineInfo.host); | ||
| this.altPortInfos[engineId] = { | ||
| from: Number(url.port), | ||
| to: port, | ||
| }; | ||
|
|
||
| url.port = port.toString(); | ||
| engineInfo.host = url.toString(); | ||
| } |
There was a problem hiding this comment.
あ、ここの関数だけは新しく作りました。
（EngineProcessManager側から叩けないといけないので）
|
@sabonerune この分け方に関して微妙な点とか思いついたらご教示いただけると･･･！ 🙇 |
|
多分分け方自体は問題ないと思います。 |
|
@sabonerune ありがとうございます！！
なるほどです！！ ちなみにそれってこのプルリクエストで絶対ないといけないとより、将来そういうのがあった方がいいよねって感じ･･･で認識合ってそうでしょうか 👀 |
|
たーーーーーぶん大丈夫だと思うのでマージしたいと思います！！レビューありがとうございます！ エンジン周りとvvpp周りを含めて管理するクラス（もしくは関数群）を作ってみようと思います！！ |
内容
EngineManagerが1000行を超えて肥えていたのが気になったので分離できないか考えてみました。
情報を管理する部分と、プロセスを管理する部分があったのでそこで分けてみました。
元コードと移動先の表
関連 Issue
の一環のリファクタリングです。
その他