Surya Hebbar has posted comments on this change. ( http://gerrit.cloudera.org:8080/22599 )
Change subject: IMPALA-13795: Support serving webUI content with gzip compression ...................................................................... Patch Set 8: (5 comments) http://gerrit.cloudera.org:8080/#/c/22599/7/be/src/util/webserver.cc File be/src/util/webserver.cc: http://gerrit.cloudera.org:8080/#/c/22599/7/be/src/util/webserver.cc@244 PS7, Line 244: const string& > Does it make sense to pass this as const reference since there to avoid dat Multiple casts(i.e. const_cast, reinterpret_cast) would be required, but it should be fine. Done. http://gerrit.cloudera.org:8080/#/c/22599/7/be/src/util/webserver.cc@291 PS7, Line 291: > I guess this is good enough for Impala's embedded webserver, but just FYI: I did not know about this standard. Thank you for the info. To make Impala's webserver fully compliant with HTTP standards, it would probably take multiple changes. If its something we use many times, I can definitely implement it for this case, or mention this in a JIRA. http://gerrit.cloudera.org:8080/#/c/22599/7/be/src/util/webserver.cc@292 PS7, Line 292: != NULL && string(accepted_encodings).find(" > If this returns non-OK status, does it make sense to log about the failure? Internally, it is already being logged, please refer to the function definition(i.e. L248, L260). Externally, according to a general HTTP server, or the standard, as the server decides the encoding, we omit the content-encoding and are serving the raw response. http://gerrit.cloudera.org:8080/#/c/22599/7/be/src/util/webserver.cc@301 PS7, Line 301: > Could it be 0 if CompressStringToBuffer() failed while calling Codec::Creat This should be "content" instead of "output". Done. http://gerrit.cloudera.org:8080/#/c/22599/7/be/src/util/webserver.cc@304 PS7, Line 304: conten > nit: I'd consult Impala's style guide regarding variable name shadowing Sorry, I seem to have forgotten to rename it to "output_str" as above. Fixed both the naming mistakes, while serving the raw output. This should fix test failures. Done. -- To view, visit http://gerrit.cloudera.org:8080/22599 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I431088a30337bbef2c8d6e16dd15fb6572db0f15 Gerrit-Change-Number: 22599 Gerrit-PatchSet: 8 Gerrit-Owner: Surya Hebbar <sheb...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <ale...@apache.org> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Joe McDonnell <joemcdonn...@cloudera.com> Gerrit-Reviewer: Riza Suminto <riza.sumi...@cloudera.com> Gerrit-Reviewer: Surya Hebbar <sheb...@cloudera.com> Gerrit-Comment-Date: Thu, 22 May 2025 19:25:40 +0000 Gerrit-HasComments: Yes
