Adar Dembo has posted comments on this change. Change subject: Add unique id generation to the client ......................................................................
Patch Set 8: (5 comments) http://gerrit.cloudera.org:8080/#/c/3077/8//COMMIT_MSG Commit Message: Line 10: an uuid. The uuid is genrated by the builder, specifically by a static Nit: generated Line 13: This also adds a test that generated 1000 clients and makes sure that Nit: "generates" (since you're writing in the present tense). Line 14: Since this test will now run all the time : we should get notified if stuff becomes flaky dues to repeated ids. Is this a real concern, though? Don't we use OidGenerators elsewhere without such tests? http://gerrit.cloudera.org:8080/#/c/3077/8/src/kudu/client/client.h File src/kudu/client/client.h: Line 255: const std::string& client_id() const; Why expose it to users at all, though? How is it useful to them? I thought it was just an implementation detail. http://gerrit.cloudera.org:8080/#/c/3077/8/src/kudu/client/client_builder-internal.h File src/kudu/client/client_builder-internal.h: Line 24: #include "kudu/util/oid_generator.h" Don't need this here; it's only used in the .cc. -- To view, visit http://gerrit.cloudera.org:8080/3077 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie752ab8337cc76c3be1536958c24a8b6c5794650 Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
