Skip to content

Comments

connpass APIの呼び出し回数を削減する#1550

Merged
yasulab merged 5 commits intocoderdojo-japan:mainfrom
takatama:multiple-series-ids-for-connpass-api
Jul 20, 2023
Merged

connpass APIの呼び出し回数を削減する#1550
yasulab merged 5 commits intocoderdojo-japan:mainfrom
takatama:multiple-series-ids-for-connpass-api

Conversation

@takatama
Copy link
Contributor

@takatama takatama commented Jul 14, 2023

connpass APIは、series_idに複数のグループをカンマ区切りで指定してイベント情報を取得できます。

グループごとにAPIを呼び出すのに比べ、呼び出し回数を大幅に削減できます。これにより、#1535 で導入したSleepを入れても、現実的な時間で情報の取得が完了します。

懸念事項が2つあります。

  1. この実装だとDojoやDojoEventServiceが大量になった場合に性能問題を引き起こす可能性があります。もっと良いやり方があればぜひご指摘ください 🙇
  2. テスト(spec)ですが、series_idが1つだけのままになっています。テストするのに適した道場があればお知らせください 🙇

@yasulab
Copy link
Member

yasulab commented Jul 14, 2023 •
edited
Loading

PR ありがとうございます! #1535 についてですが、こちらはマージ後に Rake タスクが失敗し、近日開催の道場などの各種機能が動かなくなったために一旦 Revert した、という経緯になります 😭💦 (原因調査のためコメントで TODO としていました!)

これにより、#1535 で導入したSleepを入れても、現実的な時間で情報の取得が完了します。

イイですね!! 😻🆒 開発環境では問題なく動き、本番環境では止まったので、タスクの実行時間が長くなったのが原因かなぁと考えていました 🤔💭 そうであれば本 PR で解決するかもですね...!! 👀💡

  • テスト(spec)ですが、series_idが1つだけのままになっています。テストするのに適した道場があればお知らせください 🙇

１つの Dojo が複数の series_id を持っているケースでテストしておくと大体のケースは網羅できると思うので、API 開発の背景周りにも詳しい CoderDojo Kodaira さんの seried_id でテストすると良さそうかなと思いました! (cc/ @togazo さん)

coderdojo.jp/db/dojo_event_services.yaml

Lines 511 to 518 in 6553a9d

- dojo_id: 13
name: connpass
group_id: 3809
url: https://coderdojokodaira-ninja.connpass.com/
- dojo_id: 13 #実態はGoogleフォームで管理
name: connpass
group_id: 11719
url: https://coderdojo-jp-kodaira.connpass.com/

@yasulab
Copy link
Member

yasulab commented Jul 14, 2023 •
edited
Loading

🤔.oO(今のところこんな感じでフワッと考えています)

  1. @takatama さん: テストケースを追加する
  2. @yasulab: 一旦本 PR をマージしてみる
  3. @yasulab: 本番環境でタスクを実行できるか確認する
    • @yasulab: もし動かなかったら、一旦ログを取っておく
    • @yasulab: もし動かなかった理由が明確ですぐに直せそうなら、エイヤッで直す
    • @yasulab: そうでなければ、一旦 Revert する

yasulab added a commit that referenced this pull request Jul 16, 2023
@yasulab
Add カレンダー連携 link in /events page #1547 #1550 #1552
cf3ee9a
@takatama takatama force-pushed the multiple-series-ids-for-connpass-api branch from 7668870 to 43ca5e2 Compare July 16, 2023 14:27
@takatama takatama force-pushed the multiple-series-ids-for-connpass-api branch from 43ca5e2 to 4aa0eb7 Compare July 16, 2023 14:29
@takatama
Copy link
Contributor Author

takatama commented Jul 16, 2023

@yasulab テストコードを追加しました！
「適切な道場はありますか？」と聞いてしまったのは、テストに使っていたデータhttps://coderdojo-okutama.connpass.com/が実在する道場だと思いこんでいたためです 😅okutama2を作ってしのいでいます。

@yasulab
Copy link
Member

yasulab commented Jul 16, 2023

テストコードの追加ありがとうございます...!! (＞人＜ )💖

しばらく時間が取れなさそうなので、まとまった時間が取れるときにマージ＆デプロイして、本番環境の方でもうまく動くか試してみますね! 🚀✨

@yasulab yasulab requested a review from nanophate July 18, 2023 01:00
@nanophate nanophate temporarily deployed to coderdojo-pi-multiple-s-cxelap July 19, 2023 15:51 Inactive
nanophate
nanophate approved these changes Jul 19, 2023
View reviewed changes
Copy link
Member

@nanophate nanophate left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

対応ありがとうございます！ 🙇

本番環境での問題発生を避けるために、Review App を作成して確認したところ、問題なく集計が行われている事を確認しました。また実行速度もだいぶ早くなっているようで大変嬉しい結果になっています。対応を行っていただき誠にありがとうございます。 😄 ✨

本番環境のログ: https://coderdojo.jp/events

2023-07-18 21:01:01.051 [coderdojo_japan] scheduler.8955 D, [2023-07-19T06:00:31.605584 #2] DEBUG -- :   TRANSACTION (0.7ms)  BEGIN
2023-07-18 21:01:01.051 [coderdojo_japan] scheduler.8955 UpcomingEvents aggregate
...
2023-07-18 21:06:12.562 [coderdojo_japan] scheduler.8955 近日開催イベント情報を収集しました
2023-07-18 21:06:12.643 [coderdojo_japan] scheduler.8955 D, [2023-07-19T06:06:12.629428 #2] DEBUG -- :   TRANSACTION (1.6ms)  COMMIT

Review App のログ: https://coderdojo-pi-multiple-s-cxelap.herokuapp.com/events

D, [2023-07-20T01:16:52.111982 #2] DEBUG -- :   TRANSACTION (0.6ms)  BEGIN
...
2023-07-20T01:17:21.641902  近日開催イベント情報を収集しました
D, [2023-07-20T01:17:21.734215 #2] DEBUG -- :   TRANSACTION (1.8ms)  COMMIT

@yasulab
Copy link
Member

yasulab commented Jul 20, 2023

こちら一旦マージしますね! 🚀✨
PR & Review ありがとうございました...!! (＞人＜ )✨

@yasulab yasulab merged commit 5f80b82 into coderdojo-japan:main Jul 20, 2023
@yasulab
Copy link
Member

yasulab commented Jul 20, 2023

以下、一通り実行して、本番環境でも問題なく動くことも確認しました!! 🚀 ✅ ✨
connpass API に関するタスクの改善 PR、ありがとうございます...!! 😻🆒✨

  1. @takatama さん: テストケースを追加する
  2. @yasulab: 一旦本 PR をマージしてみる
  3. @yasulab: 本番環境でタスクを実行できるか確認する
    • @yasulab: もし動かなかったら、一旦ログを取っておく
    • @yasulab: もし動かなかった理由が明確ですぐに直せそうなら、エイヤッで直す
    • @yasulab: そうでなければ、一旦 Revert する

@takatama takatama deleted the multiple-series-ids-for-connpass-api branch August 9, 2023 00:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nanophate nanophate nanophate approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@takatama @yasulab @nanophate