動画更新用API - #16
Conversation
UnABC
left a comment
UnABC
left a comment
There was a problem hiding this comment.
PRありがとう！
いくつか仕様に関して気になる点があったので確認してほしいです:pray-nya:
/api/ex-videos/sync APIに関して
- 再生時間の同期に関して
指定した動画を一括更新という仕組みになっているけど、同時に立つ配信枠は高々2～3程度だと思うからわざわざ一括で処理する必要はないかも。Youtube APIは1日あたり10,000ユニットの枠があるから、サークル内で使用する分にはAPI呼び出し回数の節約は気にしなくても大丈夫そうです。
現在の設計だとユーザー(クライアント側)が更新する動画を指定する必要があるけど、更新用のボタンやUIを新たに作ってユーザーに同期させるのはユーザーの手間が増えるから、動画アクセス時に自動で同期するようにした方がUXがいいと思います。
提案としては、Redisにクールダウン用のキーを保存するようにして、
- APIが叩かれたらRedisにキーが存在するか確認
- キーが存在しなければRedisにクールダウン用の有効期限(数十分～1時間程度)付きキーを保存して、Youtube APIで動画情報を問い合わせる
- アーカイブ済みなら動画再生時間を更新する。
という仕組みにしたらよさそう。WebSub とか使って完全に自動で更新するのもありだけどそこまで頑張らなくてもいいかも。
- タイトル・説明の同期に関して
#10 で一緒のAPIでまとめて処理した方がいいみたいなコメントをしちゃったけど上記の設計にするならAPIを分けた方がいいかも。混乱させてしまい申し訳ないです:bow-nya:
DB schemaに関して
Youtubeとかの外部統計データはplaybacQ内では使わない(再生回数等はplaybacQ内のみで集計する)方が設計の単純さや内部動画との整合性の面でもよさそうなので新たにexternal_video_*テーブルを作る必要はないかも。
|
cronなどで定期的にYoutubeのAPIを叩く仕様を勝手に想定してました。すみません。 再生時間の同期に関して
メタデータ更新用APIに関して
|
|
単一責任の原則、関心の分離の観点から両方とも分けた方がいいと思います。 |
|
了解しました |
UnABC
left a comment
UnABC
left a comment
There was a problem hiding this comment.
修正ありがとう！
ほぼ完璧だけど、DBの更新処理についてだけ確認をお願いします。
refreshExVideoDurationとrefreshExVideoMetadataではexecSqlCoroを使って生のSQLを記述していますが、他のAPIの実装との一貫性を保つためにDrogonのORM (mapper.update)を使用した方がいいと思います。
Drogonでは、セッター(setTitleやsetDuration等)を呼ぶと、内部で変更されたカラムだけを対象にしたUPDATE文を自動で生成してくれます。そのため、refreshExVideoMetadataのように手動でif (updateTitle && updateDescription)と分岐させてSQLを書く必要がなくなり、コードを簡潔に書くことができます。
また、refreshExVideoDurationではWHERE type = 'youtube live'のように状態を厳密にチェックして並行更新を防ごうとしているように見えます(この防御的な考え方自体はとても良いと思います!)が、Redisを使用したクールダウンの仕組みにより同時に処理が走ることはなく、他のAPIと並行で実行された場合でも、前述のDrogonの仕組みのおかげで意図しないカラムの巻き戻しは起きないので、ORMに任せてしまって大丈夫です。
|
確かにそうですね！修正します |
|
何度もすみません。修正したので再度レビューよろしくお願いします。 |
close #10