Todd Lipcon has posted comments on this change. Change subject: KUDU-1775 (part 1). Reject CREATE TABLE with negative or too-high replication factors ......................................................................
Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/5289/2/src/kudu/client/client-test.cc File src/kudu/client/client-test.cc: Line 3764: TEST_F(ClientTest, TestCreateTableWithBadNumReplicas) { > Would also be nice to test that, after changing the guard rail flag, we _ca I don't really see that giving us any incremental coverage, because we already test the "less than the maximum" case in every other test case where we are creating tables. Just seems to add maintenance burden of more test code. http://gerrit.cloudera.org:8080/#/c/5289/2/src/kudu/client/client.cc File src/kudu/client/client.cc: Line 592: if (data_->num_replicas_ != boost::none) { > Nothing wrong with using boost::optional here, but does this change fix a b yea, previously the client side would ignore us if we tried to set a negative replication factor, so even though we had a server-side bug, there wasn't a way to trigger it. Rather than depend on the client to do validation it's better to do the validation server side. Doing it in both places means it's harder to test the server side validation. Plus, it's better to get an error message than just ignoring them if they set an invalid replication factor. -- To view, visit http://gerrit.cloudera.org:8080/5289 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I83fedc7cb9722c049c20eb7766605fbb2f966347 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Reviewer: Will Berkeley <[email protected]> Gerrit-HasComments: Yes
