You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Alexey Serbin (Code Review)" <ge...@cloudera.org> on 2023/04/01 04:04:31 UTC

[kudu-CR] jwt: Additional test

Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19156 )

Change subject: jwt: Additional test
......................................................................


Patch Set 12:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/19156/12/src/kudu/util/CMakeLists.txt
File src/kudu/util/CMakeLists.txt:

http://gerrit.cloudera.org:8080/#/c/19156/12/src/kudu/util/CMakeLists.txt@381
PS12, Line 381:   jwt_test_certs)
Why do we need to link in jwt_test_certs into kudu_jwt_util?  Just because jwt-util-test needs those certs and also links in kudu_jwt_util?

This is a bit strange, and that's not desirable: kudu_jwt_util is then linked into the server_process library, per src/kudu/server/CMakeLists.txt

In release build, all these libraries are linked in statically, and by transitive dependencies the test-only code in jwt_test_certs.cc gets into kudu-master and kudu-tserver binaries, but that should not be like that.

Instead, maybe just link jwt_test_certs into jwt-util-test in addition to kudu_jwt_util, but don't link jwt_test_certs as a dependency into kudu_jwt_util?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1977c80b70fd9628ac800671f6cf16e9fa96c0f0
Gerrit-Change-Number: 19156
Gerrit-PatchSet: 12
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Comment-Date: Sat, 01 Apr 2023 04:04:31 +0000
Gerrit-HasComments: Yes