Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12884 )
Change subject: IMPALA-7290: part 2: Add HS2 support to Impala shell ...................................................................... Patch Set 16: (1 comment) http://gerrit.cloudera.org:8080/#/c/12884/16/common/thrift/ImpalaService.thrift File common/thrift/ImpalaService.thrift: http://gerrit.cloudera.org:8080/#/c/12884/16/common/thrift/ImpalaService.thrift@545 PS16, Line 545: // Same as HS2 CloseOperation but can return additional information. > Did you consider using the SUCCESS_WITH_INFO_STATUS/TStatus::infoMessages t I didn't think about that approach but it's an interesting idea. I initially tried to get it working without adding HS2 methods but there were enough behavioural differences (query URLs, version numbers, DML reporting) that it gave me pause. I think we should be clear about what we would be trying to achieve though, since it would affect the design and testing required. It could allow us to run impala-shell against a non-Impala HS2 implementation or against an older Impala version (with degraded functionality (i.e. observability like summary, profile, insert results, etc doesn't work). I think that is possible but would require extra testing - now we have a hard failure on PingImpalaHS2Service so no "interesting" behaviour is possible. I don't think running impala-shell against non-Impala systems is an important use case. I think running against older Impala versions would be very nice, but it would be a fair bit of effort to test (and degraded functionality could cause confusion - users might want to use an older impala-shell or beeswax in that case anyway). I think generally reducing API surface area would be nice, but explicit APIs are better than implicit APIs like data encoded in strings. -- To view, visit http://gerrit.cloudera.org:8080/12884 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6d5cc83d545aacc659523f29b1d6feed672e2a12 Gerrit-Change-Number: 12884 Gerrit-PatchSet: 16 Gerrit-Owner: Tim Armstrong <[email protected]> Gerrit-Reviewer: Fredy Wijaya <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Thomas Marshall <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Mon, 20 May 2019 18:33:28 +0000 Gerrit-HasComments: Yes
