Sailesh Mukil has posted comments on this change. Change subject: IMPALA-2904: Support INSERT and LOAD DATA on S3 and between filesystems ......................................................................
Patch Set 5: (2 comments) http://gerrit.cloudera.org:8080/#/c/2574/5/tests/common/impala_test_suite.py File tests/common/impala_test_suite.py: Line 120: generic_client > Yes, I think it would be much cleaner if we only had filesystem_client and I've left hdfs_client in places which are only HDFS specific tests. I spoke to Harrison to get a second opinion and he said it would be good to leave 'hdfs_client' in HDFS specific tests as it is, so that it's more obvious to the reader that the test is HDFS specific. http://gerrit.cloudera.org:8080/#/c/2574/5/tests/util/hdfs_util.py File tests/util/hdfs_util.py: Line 119: def exists(self, path): > I'm a bit confused. Where is super(PyWebHdfsClientWithChmod, self).exists(p Hmm, that's odd. The only reason I knew the function existed is because we already use it in our code: https://github.com/cloudera/Impala/blob/cdh5-trunk/tests/metadata/test_ddl.py#L88 It works for sure. I've tested the function itself properly. I think it complains only for 'exists()' because exists() is the only function that is "implemented" by PyWebHdfsClient with the same signature as the declaration in BaseFileSystem. All the other functions have variable arguments, so they're not really "implemented" by PyWebHdfsClient. P.S: PyWebHdfsClientWithChmod inherits from PyWebHdfsClient and BaseFileSystem. -- To view, visit http://gerrit.cloudera.org:8080/2574 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I94e15ad67752dce21c9b7c1dced6e114905a942d Gerrit-PatchSet: 5 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Sailesh Mukil <[email protected]> Gerrit-Reviewer: Henry Robinson <[email protected]> Gerrit-Reviewer: Michael Brown <[email protected]> Gerrit-Reviewer: Mostafa Mokhtar <[email protected]> Gerrit-Reviewer: Sailesh Mukil <[email protected]> Gerrit-Reviewer: Taras Bobrovytsky <[email protected]> Gerrit-HasComments: Yes
