Matthew Jacobs has posted comments on this change. Change subject: IMPALA-5333: Add support for Impala to work with ADLS ......................................................................
Patch Set 1: (12 comments) http://gerrit.cloudera.org:8080/#/c/6910/1//COMMIT_MSG Commit Message: PS1, Line 18: This client seems to have some consistency : issues. Is this a known issue? Any tickets to reference for this? http://gerrit.cloudera.org:8080/#/c/6910/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java: PS1, Line 774: S3 update http://gerrit.cloudera.org:8080/#/c/6910/1/infra/python/deps/compiled-requirements.txt File infra/python/deps/compiled-requirements.txt: Line 35: pycparser == 2.17 IIRC the indentation is meant to indicate dependency. In the Kudu case below, the comment is meant to be the 'top level'. Just leave a comment above this indicating # Required for adls http://gerrit.cloudera.org:8080/#/c/6910/1/tests/metadata/test_ddl.py File tests/metadata/test_ddl.py: PS1, Line 61: seems It'd be good to make a conclusive statement. I assume the consistency model should be well defined somewhere. nit: "so skip for it" sounds awkward Line 156: print self.filesystem_client.ls( remove? Line 304: # The ADLS client seems to be inconsistent too. So skip for it. same as above http://gerrit.cloudera.org:8080/#/c/6910/1/tests/metadata/test_stale_metadata.py File tests/metadata/test_stale_metadata.py: PS1, Line 22: SkipIfADLS not used http://gerrit.cloudera.org:8080/#/c/6910/1/tests/query_test/test_insert_behaviour.py File tests/query_test/test_insert_behaviour.py: Line 48: @SkipIfADLS.slow_client why does this work on s3 but not adls? http://gerrit.cloudera.org:8080/#/c/6910/1/tests/query_test/test_insert_parquet.py File tests/query_test/test_insert_parquet.py: Line 164: @SkipIfADLS.hdfs_block_size does this work on s3? http://gerrit.cloudera.org:8080/#/c/6910/1/tests/util/adls_util.py File tests/util/adls_util.py: Line 26: class ADLSClient(BaseFilesystem): nit: newline above this Line 38: f.write(file_data) this has a return value, #bytes I think, it'd be good to check, e.g. log an error/return false if it is less than expected. PS1, Line 47: # The ADLS Python client doesn't support cp() yet, so we have to download and : # reupload to the destination. ? http://azure-datalake-store.readthedocs.io/en/latest/api.html#azure.datalake.store.core.AzureDLFileSystem.cp -- To view, visit http://gerrit.cloudera.org:8080/6910 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic56b9988b32a330443f24c44f9cb2c80842f7542 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Sailesh Mukil <[email protected]> Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-HasComments: Yes
