CodiumAI-Agent commented on PR #9332: URL: https://github.com/apache/incubator-gluten/pull/9332#issuecomment-2804521704
## PR Code Suggestions ✨ <!-- abf496d --> <table><thead><tr><td><strong>Category</strong></td><td align=left><strong>Suggestion </strong></td><td align=center><strong>Impact</strong></td></tr><tbody><tr><td rowspan=2>General</td> <td> <details><summary>Ensure correct trailing slash</summary> ___ **Verify that the URL returned by hdfsHelper.getHdfsUrl includes a trailing slash when <br>needed, so that subsequent file path concatenations work as expected.** [backends-clickhouse/src/test/scala/org/apache/gluten/execution/GlutenClickHouseExcelFormatSuite.scala [1475]](https://github.com/apache/incubator-gluten/pull/9332/files#diff-a36a78c194396d3b99ef15a1be5b8619f11edb55ece99480b967dde5d0e8ee43R1475-R1475) ```diff -val tablePath = hdfsHelper.getHdfsUrl(s"$SPARK_DIR_NAME/write_into_hdfs/") +val path = hdfsHelper.getHdfsUrl(s"$SPARK_DIR_NAME/write_into_hdfs") +val tablePath = if (path.endsWith("/")) path else s"$path/" ``` <details><summary>Suggestion importance[1-10]: 6</summary> __ Why: The suggestion improves robustness by ensuring the trailing slash is present for proper path concatenation. Although the original code already includes a trailing slash, the added check can prevent potential issues if the helper's return value ever varies. </details></details></td><td align=center>Low </td></tr><tr><td> <details><summary>Normalize Minio endpoint URL</summary> ___ **Adjust URL concatenation to avoid duplicate slashes by trimming the trailing slash <br>from MINIO_ENDPOINT before forming wholePath.** [backends-clickhouse/src/test/scala/org/apache/gluten/utils/MinioTestHelper.scala [96]](https://github.com/apache/incubator-gluten/pull/9332/files#diff-9c9cfdcecec7a203ecde18cc51c33062b5d458a815e86a286dfcd14ae986f908R96-R96) ```diff -val wholePath: String = MINIO_ENDPOINT + BUCKET_NAME + "/" +val wholePath: String = s"${MINIO_ENDPOINT.stripSuffix("/")}/$BUCKET_NAME/" ``` <details><summary>Suggestion importance[1-10]: 6</summary> __ Why: This suggestion refines the URL formation by removing potential duplicate slashes, making the code more resilient to different endpoint formats. The change is minor but can help avoid errors in environments where MINIO_ENDPOINT might not already include a trailing slash. </details></details></td><td align=center>Low </td></tr></tr></tbody></table> -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
