radford nguyen has posted comments on this change. ( http://gerrit.cloudera.org:8080/13352 )
Change subject: IMPALA-8473: publish lineage info via hook ...................................................................... Patch Set 17: (15 comments) still one more iteration needed http://gerrit.cloudera.org:8080/#/c/13352/16//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/13352/16//COMMIT_MSG@38 PS16, Line 38: to every hook to execute asynchronously. In other words, > Nit: spell out IOW to help future non-native English speakers good point; done http://gerrit.cloudera.org:8080/#/c/13352/17/fe/src/main/java/org/apache/impala/hooks/QueryExecHook.java File fe/src/main/java/org/apache/impala/hooks/QueryExecHook.java: http://gerrit.cloudera.org:8080/#/c/13352/17/fe/src/main/java/org/apache/impala/hooks/QueryExecHook.java@25 PS17, Line 25: public interface QueryExecHook { Before we merge this, are we happy with this name? The backend seems to use the terminology "query event", e.g. `(*request_state)->query_events()` https://github.com/apache/impala/blob/3.2.0/be/src/service/impala-server.cc#L934 Perhaps we should rename this to `QueryEventHook` and likewise in other similar places. http://gerrit.cloudera.org:8080/#/c/13352/16/tests/hooks/test_hooks.py File tests/hooks/test_hooks.py: http://gerrit.cloudera.org:8080/#/c/13352/16/tests/hooks/test_hooks.py@299 PS16, Line 299: pass > If you make LOG_DIR unique you could leave it around which might help someo much better; done http://gerrit.cloudera.org:8080/#/c/13352/17/tests/hooks/test_hooks.py File tests/hooks/test_hooks.py: http://gerrit.cloudera.org:8080/#/c/13352/17/tests/hooks/test_hooks.py@30 PS17, Line 30: from getpass import getuser > flake8: E402 module level import not at top of file Done http://gerrit.cloudera.org:8080/#/c/13352/17/tests/hooks/test_hooks.py@31 PS17, Line 31: from ImpalaService import ImpalaHiveServer2Service > flake8: E402 module level import not at top of file Done http://gerrit.cloudera.org:8080/#/c/13352/17/tests/hooks/test_hooks.py@32 PS17, Line 32: from TCLIService import TCLIService > flake8: E402 module level import not at top of file Done http://gerrit.cloudera.org:8080/#/c/13352/17/tests/hooks/test_hooks.py@33 PS17, Line 33: from thrift.transport.TSocket import TSocket > flake8: E402 module level import not at top of file Done http://gerrit.cloudera.org:8080/#/c/13352/17/tests/hooks/test_hooks.py@34 PS17, Line 34: from thrift.transport.TTransport import TBufferedTransport > flake8: E402 module level import not at top of file Done http://gerrit.cloudera.org:8080/#/c/13352/17/tests/hooks/test_hooks.py@35 PS17, Line 35: from thrift.protocol import TBinaryProtocol > flake8: E402 module level import not at top of file Done http://gerrit.cloudera.org:8080/#/c/13352/17/tests/hooks/test_hooks.py@36 PS17, Line 36: from tests.common.custom_cluster_test_suite import CustomClusterTestSuite > flake8: E402 module level import not at top of file Done http://gerrit.cloudera.org:8080/#/c/13352/17/tests/hooks/test_hooks.py@37 PS17, Line 37: from tests.common.file_utils import assert_file_in_dir_contains,\ > flake8: E402 module level import not at top of file Done http://gerrit.cloudera.org:8080/#/c/13352/17/tests/hooks/test_hooks.py@37 PS17, Line 37: from tests.common.file_utils import assert_file_in_dir_contains,\ > flake8: F401 'tests.common.file_utils.assert_no_files_in_dir_contain' impor Done http://gerrit.cloudera.org:8080/#/c/13352/17/tests/hooks/test_hooks.py@205 PS17, Line 205: def __wait_for_file(self, filepath, timeout_s=10): Should this go in common/file_utils.py? I didn't put it there now because it's just a first-pass, quick-and-dirty implementation http://gerrit.cloudera.org:8080/#/c/13352/17/tests/hooks/test_hooks.py@217 PS17, Line 217: class TestHooksStartupFail(CustomClusterTestSuite): > flake8: E302 expected 2 blank lines, found 1 Done http://gerrit.cloudera.org:8080/#/c/13352/17/tests/hooks/test_hooks.py@248 PS17, Line 248: d > flake8: E304 blank lines found after function decorator Done -- To view, visit http://gerrit.cloudera.org:8080/13352 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I23a896537a98bfef07fb27c70e9a87c105cd77a1 Gerrit-Change-Number: 13352 Gerrit-PatchSet: 17 Gerrit-Owner: radford nguyen <radford.ngu...@gmail.com> Gerrit-Reviewer: Andrew Sherman <asher...@cloudera.com> Gerrit-Reviewer: Anonymous Coward (498) Gerrit-Reviewer: Austin Nobis <ano...@cloudera.com> Gerrit-Reviewer: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: radford nguyen <radford.ngu...@gmail.com> Gerrit-Comment-Date: Wed, 22 May 2019 20:11:33 +0000 Gerrit-HasComments: Yes
