Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21581 )
Change subject: IMPALA-13115: Add query id to message of UNREACHABLE_IMPALADS ...................................................................... Patch Set 2: (2 comments) This is something we could be more consistent about. Thanks. http://gerrit.cloudera.org:8080/#/c/21581/2/tests/custom_cluster/test_process_failures.py File tests/custom_cluster/test_process_failures.py: http://gerrit.cloudera.org:8080/#/c/21581/2/tests/custom_cluster/test_process_failures.py@156 PS2, Line 156: UNREACHABLE_IMPALADS = 125 You can get this with from ErrorCodes.ttypes import TErrorCode ... TErrorCode.UNREACHABLE_IMPALADS as seen in test_custom_statestore.py. http://gerrit.cloudera.org:8080/#/c/21581/2/tests/custom_cluster/test_process_failures.py@157 PS2, Line 157: error_state = get_error_formatter(UNREACHABLE_IMPALADS).format(query_id, "") A part of me prefers having the explicit string here, as it makes it easier to visually confirm that the format is what we expect. I'm not opposed to this change however, as get_error_formatter at least provides a separate method of constructing the error string so unlikely to make the same kind of mistakes as we might make in C++ or Java formatting. -- To view, visit http://gerrit.cloudera.org:8080/21581 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I72260687ad395f97972e65ffcb47846e9de75bfb Gerrit-Change-Number: 21581 Gerrit-PatchSet: 2 Gerrit-Owner: Xuebin Su <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Comment-Date: Fri, 12 Jul 2024 17:52:52 +0000 Gerrit-HasComments: Yes
