AWS CodeGuruでPythonのコードを自動レビューしてもらおう

おもにクラウドの話してます - 広島 #2 での発表資料です
https://omoni-cloud.connpass.com/event/320415/

Transcript

1. AWS CodeGuruでPythonのコードを ⾃動レビューしてもらおう 2024.06.28 SATOSHI KANEYASU
2. 2 ⾃⼰紹介 ⽒名︓兼安 聡 所属︓株式会社サーバーワークス 在住︓広島（フルリモート） 担当︓DevOps、PM、SM 2024 Japan AWS Top Engineers (Database) 2024 Japan AWS All Certifications Engineers 認定スクラムマスター X︓@satoshi256kbyte
3. 3 今回のお話 レビュアーの負担を軽減するために、機械にコードレビュー してもらおうという試みです Amazon CodeGuruReviewerを使⽤してみます Glueじゃなくて、Guruです すいません、データベースネタは⽤意できませんでした
4. 4 Amazon CodeGuruReviewerの料⾦ リポジトリサイズ (AWS アカウントごと) ⽉額料⾦ オンボーディングリポジトリの最初の 10 万⾏のコード 10.00$ オンボーディングされたリポジトリの追加の 10 万⾏のコード (最近接の 10 万に切り上げられます) 30.00$ ※東京リージョンの価格です。 参考
5. 5 料⾦の発⽣の仕⽅ Amazon CodeGuruReviewerの利⽤開始をしたその⽇に、 １ヶ⽉分の料⾦がガン︕と発⽣します
6. 6 使い⽅ – まずはリポジトリを紐付けます 今回は AWS CodeCommitを選択
7. 7 使い⽅ – 紐づけたリポジトリ全体に対してレビュー レビュー対象とするブランチを 選択します
8. 8 レビュー結果 30件程度⾒つかりました 次⾴からレビュー内容を⾒て⾏きます なお、レビュー対象のソースは、 事前にlintは通してあります ⾏番号まで出ます
9. 9 レビュー結果① To check if a container or sequence (string, list, tuple) is empty, use if not val. Do not compare its length using if len(val) == 0 コンテナやシーケンス（⽂字列、リスト、タプル）が空かどうかをチェックするには、if not val を使 ⽤します。if len(val) == 0 を使って⻑さを⽐較しないでください。 if len(val) > 0: 空判定に指摘が⼊りました。 ⾔われてみれば妥当ですが、細かい指摘ですね。 原⽂ ⽇本語訳
10. 10 レビュー結果② To return a single item from a list, we recommend using the list.index() method or indexing [] operator instead of a for loop. Iteration when only one item is needed from list is inefficient and can make your code difficult リストから単⼀の項⽬を返すには、forループの代わりにlist.index()メソッドまたはindexing []演算⼦を使⽤ することをお勧めします。 リストから必要な項⽬が 1 つだけの場合に繰り返し処理を⾏うと、⾮効率的でコードが読みにくくなります。 data_map: dict[str, list[str]] = {address: [key for key, value in host_mapping.items() if value == address] for address in address_list} 原⽂ ⽇本語訳 dictの再構成に指摘が⼊りました。 こちら確かにindexの⽅が効率が良いようです。 このコードは確かに読みにくく、良い指摘をもらえたと感じます。
11. 11 レビュー結果③ This function calls 16 other functions. By comparison, 98% of the functions in the CodeGuru reference dataset use fewer. This indicates the function is highly coupled with other functions. A function that is highly coupled with other functions is difficult to understand and its behavior might change unexpectedly when one of its referenced functions is updated. High coupling could also increase the integration test complexity, maintenance cost and technical debt. We recommend that you simplify this function or break it into multiple functions. For example, consider extracting the code blocks on lines 103-122, lines 125-131, lines 135-141, lines 146-155, lines 160-193 into separate functions. The high number of functions called inside a function indicates that it is highly coupled with other functions. A function that is highly coupled with other functions is difficult to understand and its behavior might change unexpectedly when one of its called functions is updated. High coupling can also increase the integration test complexity, maintenance cost and technical debt. この関数は16の他の関数を呼び出している。これに対して、CodeGuruのリファレンス・データセットにある関数の98%は、 より少ない数しか使っていません。これは、この関数が他の関数と⾼度に結合していることを⽰しています。他の関数と⾼ 度に結合している関数は、理解するのが難しく、参照されている関数の1つが更新されると、その動作が予期せず変更される 可能性があります。また、⾼度な結合は、統合テストの複雑さ、保守コスト、技術的負債を増加させる可能性があります。 この関数を単純化するか、複数の関数に分割することを推奨します。例えば、103⾏⽬から122⾏⽬、125⾏⽬から131⾏⽬、 135⾏⽬から141⾏⽬、146⾏⽬から155⾏⽬、160⾏⽬から193⾏⽬のコードブロックを別の関数に分割することを検討し てください。 関数の内部で呼び出される関数の数が多いということは、その関数が他の関数と⾼度に結合してい ることを⽰しています。 他の関数と⾼度に結合している関数は理解しづらく、呼び出された関数の1つが更新されると、その振る舞いが予期せず変化 する可能性があります。⾼度な結合は、統合テストの複雑さ、保守コスト、技術的負債を増加させる可能性もあります。 原⽂ ⽇本語訳
12. 12 レビュー結果③ レビュー結果③のコードはお⾒せできませんが、確かにメソッドが⼤きいです。 分割した⽅がよいという指摘をしてくれました。
13. 13 ちょっとお遊び この関数は設計の⾒本市で⾦賞を取れるほど複雑で、保守性と技術的負債で逆に記録を更新しています。 16もの他の関数を呼び出しており、これほど結合が強いと、⼩さな変更が⼤惨事を引き起こすリスクが 常に付きまとうことになります。 このごちゃごちゃしたコードを何とか整理し、複数の⼩さな関数に分割することを強く推奨します。将 来の⾃分（または他の誰か）がこのコードを⾒たときに、感謝されるよう努めましょう。さもなければ、 この技術的負債がいつか痛いしっぺ返しを⾷らわせることになります。 指摘内容が硬いのでChatGPTに 「レビュー結果を簡潔かつできるだけ⾟辣な⽇本語にしてください」と 依頼してみました
14. 14 ちょっとお遊び まわりくどくて⾟辣に感じませんでしたw
15. 15 IaCもレビューしてくれる Ensure that S3 buckets have cross-region replication enabled. By enabling cross-region replication for your S3 buckets, you create redundant copies of your data in different AWS regions. This helps improve data durability and availability, as well as provides protection against regional outages. It also allows you to comply with data residency requirements by replicating data to specific regions. S3 buckets have cross-region replication not enabled. Ensure that S3 buckets have cross- region replication enabled. S3バケットでクロスリージョンレプリケーションが有効になっていることを確認する。S3バケットのク ロスリージョンレプリケーションを有効にすることで、異なるAWSリージョンにデータの冗⻑コピーを 作成する。これは、データの耐久性と可⽤性を向上させ、リージョン停⽌に対する保護を提供するのに 役⽴ちます。また、特定のリージョンにデータをレプリケートすることで、データレジデンシー要件に 準拠することもできます。 S3バケットのクロスリージョンレプリケーションが有効になっていない。S3バケットでクロスリージョ ンレプリケーションが有効になっていることを確認してください。 ⽇本語訳 原⽂
16. 16 IaCもレビューしてくれる IaCもレビューしてくれるのはありがたいですが、 少々過剰品質を求めてるレビューに⾒えます。
17. 17 増分コードレビュー プルリクエストに対して⾃動でレビュー プルリクエスト作成 数⼗秒後、CodeGuru側でレビュー開始
18. 18 増分コードレビュー レビュー結果はプルリクエストの「アクティビティ」に表⽰
19. 19 CodeGuruのレビュー結果の感想 lintより賢く、細かいところを突いてくれる ⼤局的な視点、業務的な視点でのレビューは無理 IaCのレビューをしてくれるのはありがたいが、過剰なこと を⾔ってくることも レビュアーの代替にはならないが、負荷軽減にはなりそう CodeGuruのレビューがオールクリアでないと、 マージしてはならないという運⽤は違うかと
20. 20 考えられる運⽤ 対応必須とはしないぐらいかと • PR作成者が返信で先にコメント・リアクションをしておく • 必要でない指摘と思うならその旨書いて済ませてよいかと • 増分レビューなら数はそこまで多くなく、コメント⾃体はそこまで負担 にはならないと予想
21. 21 GitHubとの接続 可能のはずだが、接続の際に組織名が出てくるのが気になる 現在調査中、慎重に進めた⽅が良さそう
22. 22 今後考えて⾏きたいスタイル 設計 実装 ソース レビュー ⼈⼒レビュー Amazon Q Developer ⼈⼒レビュー Amazon CodeGuru GitHub Copilot
23. 23 今後考えて⾏きたいスタイル 設計 実装 ソース レビュー ⼈⼒レビュー Amazon Q Developer ⼈⼒レビュー Amazon CodeGuru GitHub Copilot ここは現場の事情に応じて 選びたい
24. 24 今⽇はここまでです また報告します