Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/17121 )
Change subject: IMPALA-7712: Support Google Cloud Storage ...................................................................... Patch Set 10: Code-Review+2 (1 comment) Given the analysis in IMPALA-10563, it seems fine to disable those test cases for now. See my note about IMPALA-10579. I think it is ok to include this partial fix, as it seems better than what we have right now. If IMPALA-10579 was landing very soon, I would be ok with removing this piece of the fix and relying on IMPALA-10579. This change makes sense to me, and it is good to have the GCS support land. http://gerrit.cloudera.org:8080/#/c/17121/10/fe/src/main/java/org/apache/impala/common/FileSystemUtil.java File fe/src/main/java/org/apache/impala/common/FileSystemUtil.java: http://gerrit.cloudera.org:8080/#/c/17121/10/fe/src/main/java/org/apache/impala/common/FileSystemUtil.java@713 PS10, Line 713: /** : * Wrapper around FileSystem.listStatusIterator() to make sure the path exists. : * : * @throws FileNotFoundException if <code>p</code> does not exist : * @throws IOException if any I/O error occurredd : */ : public static RemoteIterator<FileStatus> listStatusIterator(FileSystem fs, Path p) : throws IOException { : RemoteIterator<FileStatus> iterator = fs.listStatusIterator(p); : // Some FileSystem implementations like GoogleHadoopFileSystem doesn't check : // existence of the start path when creating the RemoteIterator. Instead, their : // iterators throw the FileNotFoundException in the first call of hasNext() when : // the start path doesn't exist. Here we call hasNext() to ensure start path exists. : iterator.hasNext(); : return iterator; This code will be replaced by IMPALA-10579. I'm guessing that the thought here is that this is better than what we have, and the fuller fix will come from IMPALA-10579. -- To view, visit http://gerrit.cloudera.org:8080/17121 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia91ec956de3b620cccf6a1244b56b7da7a45b32b Gerrit-Change-Number: 17121 Gerrit-PatchSet: 10 Gerrit-Owner: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Joe McDonnell <joemcdonn...@cloudera.com> Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Comment-Date: Fri, 12 Mar 2021 18:42:52 +0000 Gerrit-HasComments: Yes