Attila Bukor has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16658 )

Change subject: KUDU-3210 Increase key size in tests and EMC
......................................................................


Patch Set 13:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/16658/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/16658/3//COMMIT_MSG@11
PS3, Line 11: This commit introduces a
> Thank you for benchmarking those!  I guess perf statistics for ctest itself
As agreed on the call yesterday, I put all these behind an environment variable.


http://gerrit.cloudera.org:8080/#/c/16658/3/src/kudu/master/master-test.cc
File src/kudu/master/master-test.cc:

http://gerrit.cloudera.org:8080/#/c/16658/3/src/kudu/master/master-test.cc@a1970
PS3, Line 1970:
> Why don't to keep this comment explaining the semantics behind the magic nu
Done


http://gerrit.cloudera.org:8080/#/c/16658/12/src/kudu/mini-cluster/external_mini_cluster.cc
File src/kudu/mini-cluster/external_mini_cluster.cc:

http://gerrit.cloudera.org:8080/#/c/16658/12/src/kudu/mini-cluster/external_mini_cluster.cc@1011
PS12, Line 1011: are
> nit: are not ?  Or that was a part of an incomplete sentence?
Done


http://gerrit.cloudera.org:8080/#/c/16658/12/src/kudu/util/test_util.cc
File src/kudu/util/test_util.cc:

http://gerrit.cloudera.org:8080/#/c/16658/12/src/kudu/util/test_util.cc@119
PS12, Line 119:     // necessary to override it to use the key length specified 
above (which are
              :     // considered lax and don't work in case of security level 
2 or higher).
              :     flags_for_tests.emplace("openssl_security_level_override", 
"0");
> nit: do you mind moving this to precede the line with openssl_security_leve
Done


http://gerrit.cloudera.org:8080/#/c/16658/12/src/kudu/util/test_util.cc@129
PS12, Line 129: }
> nit: it seems unnecessary extra indent was added
Done


http://gerrit.cloudera.org:8080/#/c/16658/12/src/kudu/util/test_util.cc@133
PS12, Line 133:   FLAGS_log_dir = GetTestDataDirectory();
> I guess this needs to be set only once, not sure we need to place it under
Done



--
To view, visit http://gerrit.cloudera.org:8080/16658
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I45b96e0b4499ea1d80db5871a529c732ad41220e
Gerrit-Change-Number: 16658
Gerrit-PatchSet: 13
Gerrit-Owner: Attila Bukor <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Attila Bukor <[email protected]>
Gerrit-Reviewer: Grant Henke <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wenzhe Zhou <[email protected]>
Gerrit-Comment-Date: Tue, 03 Nov 2020 19:46:10 +0000
Gerrit-HasComments: Yes

Reply via email to