Alexey Serbin has posted comments on this change.

Change subject: Expose row format flags in KuduScanner
......................................................................


Patch Set 10:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/6624/9/src/kudu/client/scanner-internal.cc
File src/kudu/client/scanner-internal.cc:

PS9, Line 546: LOG(FATAL)
Maybe, return Status from this method instead of aborting here?  That would be 
cleaner, IMO: all other places where the method is used return Status 
themselves, so it's easy to update.

BTW, this method is also used by the kudu tool -- not crashing in such cases 
would give better impression for the end-users, I think.


http://gerrit.cloudera.org:8080/#/c/6624/10/src/kudu/tserver/scanners-test.cc
File src/kudu/tserver/scanners-test.cc:

PS10, Line 23: #include "kudu/tserver/tserver.pb.h"
             : #include "kudu/tserver/scanner_metrics.h"
nit: re-order these lines to be sorted properly.


-- 
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: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[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

Reply via email to