Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/19271 )
Change subject: KUDU-3423 Add support for Ranger KMS HA ...................................................................... Patch Set 4: (6 comments) http://gerrit.cloudera.org:8080/#/c/19271/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19271/4//COMMIT_MSG@16 PS4, Line 16: If a server : responds with an error > Is Ranges KMS able to respond with "503 Service Unavailable" or something l Not sure if it's able to, I thought this is the least opinionated approach. If it's not enough, we can always change it later. http://gerrit.cloudera.org:8080/#/c/19271/4/src/kudu/fs/fs_manager.cc File src/kudu/fs/fs_manager.cc: http://gerrit.cloudera.org:8080/#/c/19271/4/src/kudu/fs/fs_manager.cc@141 PS4, Line 141: " " > nit: could drop the space and join two parts of the string? Done http://gerrit.cloudera.org:8080/#/c/19271/4/src/kudu/fs/fs_manager.cc@149 PS4, Line 149: FLAGS_ranger_kms_url.empty() > Since now a list of comma-separated strings can be provided as value for -- Done http://gerrit.cloudera.org:8080/#/c/19271/4/src/kudu/util/curl_util.h File src/kudu/util/curl_util.h: PS4: > Consider separating this newly added {fetch,post}-with-failover functionali Thought about that, but we don't have positive tests for curl-util at all right now. Do you think it's worth adding tests using our webserver somehow? http://gerrit.cloudera.org:8080/#/c/19271/4/src/kudu/util/curl_util.h@61 PS4, Line 61: // one in the list. > Maybe, also mention that it stops at the first successful fetch attempt and Done http://gerrit.cloudera.org:8080/#/c/19271/4/src/kudu/util/curl_util.h@78 PS4, Line 78: Status PostToURL(const std::vector<std::string>& urls, > ditto: maybe mention what happens on the fist successful post Done -- To view, visit http://gerrit.cloudera.org:8080/19271 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibef941ed20eda1f4e624c2c7e16ca7955af570d8 Gerrit-Change-Number: 19271 Gerrit-PatchSet: 4 Gerrit-Owner: Attila Bukor <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Zoltan Chovan <[email protected]> Gerrit-Comment-Date: Thu, 24 Nov 2022 17:03:58 +0000 Gerrit-HasComments: Yes
