CodiumAI-Agent commented on PR #9332: URL: https://github.com/apache/incubator-gluten/pull/9332#issuecomment-2804515187
## PR Reviewer Guide ๐ Here are some key observations to aid the review process: <table> <tr><td> **๐ซ Ticket compliance analysis ๐ถ** **[6867](https://github.com/apache/incubator-gluten/issues/6867) - Partially compliant** Compliant requirements: - Support parquet table creation on minio by accepting http:// endpoints - Integration of helper classes (HDFSTestHelper, MinioTestHelper, CacheTestHelper) for reset and cleanup in tests Non-compliant requirements: - None identified Requires further human verification: - Validate that all caching and file system configurations are fully tested in various test suites </td></tr> <tr><td>โฑ๏ธ <strong>Estimated effort to review</strong>: 4 ๐ต๐ต๐ต๐ตโช</td></tr> <tr><td>๐งช <strong>PR contains tests</strong></td></tr> <tr><td>๐ <strong>No security concerns identified</strong></td></tr> <tr><td>โก <strong>Recommended focus areas for review</strong><br><br> <a href='https://github.com/apache/incubator-gluten/pull/9332/files#diff-6104e34eb10b34c27f3475456ae388c610d7067e1bf284d86ade81bb2ef0cf76R528-R533'><strong>Endpoint Format Check</strong></a><br>The new code in the S3 read buffer builder now checks if the endpoint starts with "http://"; or "https://";. Verify that this logic correctly handles all expected endpoint formats without breaking existing S3 configurations. <details><summary><a href='https://github.com/apache/incubator-gluten/pull/9332/files#diff-7e6270bad67b7a118e74b5908db203fa5db26c1baefeaaee17ac79f999c2811dR216-R225'><strong>Macro and Buffer Creation</strong></a> The modifications under the USE_HDFS conditional and changes to the build methods introduce alternate paths in buffer creation. Ensure that the conditional compilation correctly covers both HDFS and non-HDFS scenarios and that the new lambda signatures meet the intended behaviors. </summary> ```c++ start_end.second); #if USE_HDFS /// If read buffer doesn't support right bounded reads, wrap it with BoundedReadBuffer to enable right bounded reads. if (dynamic_cast<DB::ReadBufferFromHDFS *>(read_buffer.get()) || dynamic_cast<DB::AsynchronousReadBufferFromHDFS *>(read_buffer.get()) || dynamic_cast<DB::ReadBufferFromFile *>(read_buffer.get())) read_buffer = std::make_unique<DB::BoundedReadBuffer>(std::move(read_buffer)); #else if (dynamic_cast<DB::ReadBufferFromFile *>(read_buffer.get())) read_buffer = std::make_unique<DB::BoundedReadBuffer>(std::move(read_buffer)); #endif ``` </details> </td></tr> </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]
