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 2:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/22599/2/be/src/util/codec.cc
File be/src/util/codec.cc:

http://gerrit.cloudera.org:8080/#/c/22599/2/be/src/util/codec.cc@97
PS2, Line 97: Status Codec::CreateCompressor(MemPool* mem_pool, bool reuse, 
const CodecInfo& codec_info,
            :     scoped_ptr<Codec>* compressor) {
> While on this, please change NULL to nullptr there at runtime-profile.cc:16
Done


http://gerrit.cloudera.org:8080/#/c/22599/2/be/src/util/codec.cc@105
PS2, Line 105:       compressor->reset(new GzipCompressor(GzipCompressor::GZIP, 
mem_pool, reuse,
             :           codec_info.compression_level_));
> CodecInfo's compression_level_ defaults to 0, which would turn off compress
Done


http://gerrit.cloudera.org:8080/#/c/22599/1/be/src/util/compress.h
File be/src/util/compress.h:

http://gerrit.cloudera.org:8080/#/c/22599/1/be/src/util/compress.h@66
PS1, Line 66: int compression_level_
> >  ... if there is a reason to circumvent the compression_level_ provided b
Done


http://gerrit.cloudera.org:8080/#/c/22599/2/be/src/util/compress.cc
File be/src/util/compress.cc:

http://gerrit.cloudera.org:8080/#/c/22599/2/be/src/util/compress.cc@49
PS2, Line 49:   DCHECK_IN_RANGE(compression_level, -1, 9);
> We don't want to hard-code these values. There are macros in zlib.h that co
Done


http://gerrit.cloudera.org:8080/#/c/22599/2/be/src/util/compress.cc@334
PS2, Line 334:     DCHECK_IN_RANGE(clevel, -7, 29);
> We don't want to hard-code these values. zstd.h has functions that return t
Done


http://gerrit.cloudera.org:8080/#/c/22599/2/tests/webserver/test_web_pages.py
File tests/webserver/test_web_pages.py:

http://gerrit.cloudera.org:8080/#/c/22599/2/tests/webserver/test_web_pages.py@83
PS2, Line 83: "
> Please address this.
Done


http://gerrit.cloudera.org:8080/#/c/22599/3/tests/webserver/test_web_pages.py
File tests/webserver/test_web_pages.py:

http://gerrit.cloudera.org:8080/#/c/22599/3/tests/webserver/test_web_pages.py@85
PS3, Line 85:
> flake8: E501 line too long (93 > 90 characters)
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: 2
Gerrit-Owner: Surya Hebbar <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Joe McDonnell <[email protected]>
Gerrit-Reviewer: Riza Suminto <[email protected]>
Gerrit-Reviewer: Surya Hebbar <[email protected]>
Gerrit-Comment-Date: Wed, 14 May 2025 16:26:38 +0000
Gerrit-HasComments: Yes

Reply via email to