David Ribeiro Alves has posted comments on this change. Change subject: WIP: Expose a way to set "advanced" non-types scan options ......................................................................
Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/6624/4//COMMIT_MSG Commit Message: PS4, Line 15: without "polluting" the api and in such a way that we can remove : support for this in the future. > I agree with the first portion of the soap box - I would prefer an API that Thanks for weighing in. I have nothing against the enum, personally, just seems limiting from an API perspective as it doesn't allow to set a value and thus reduces the usefulness of the api itself. for instance say that we would want in the future to enable users to set the size of the row block for scans, this would need a value. If we're going the enum route maybe we should just have "pad_timestamp_16_bytes" getters and setters in KuduScanner and be done with it. Regarding the second part of the soapbox, I agree with todd. As long as it's pretty clear what is the api contract, from an evolution perspective, then users can choose to take in the risk or not. Finally I would say that while we might not easily accept these kinds from any random contributor I would say we would definitely consider them seriously for any big Kudu consumer, be it Impala or Spark or any other well known compute/query layer. -- To view, visit http://gerrit.cloudera.org:8080/6624 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I043b6514dc5fc307fc9c94eb41f3ae79796ba273 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Jean-Daniel Cryans <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
