Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17267 )
Change subject: [master][tool] KUDU-2181 Tool to orchestrate adding a master ...................................................................... Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/17267/2/src/kudu/tools/tool_action_common.cc File src/kudu/tools/tool_action_common.cc: http://gerrit.cloudera.org:8080/#/c/17267/2/src/kudu/tools/tool_action_common.cc@770 PS2, Line 770: CHECK(Env::Default()->FileExists(tool_abs_path)) CHECK() was fine for test envrionments where the absence of the 'kudu' binary would be a programmatic error since the testing environment is pretty much 'fixed': we always have the binary over there and bring it along in dist-test. However, in field scenarios, the binary might be renamed into 'kudu.old' or something else. Crashing on such a condition might be frustrating (or look too funny) for users. Once function gets more 'external' exposure, consider handling the case of absent binary gracefully, i.e. report a proper error and don't crash at once. Maybe, there should be even an option to specify a 'kudu' binary name? -- To view, visit http://gerrit.cloudera.org:8080/17267 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8f99cf2b3f1738c4127c7e7288beab61daf42e7b Gerrit-Change-Number: 17267 Gerrit-PatchSet: 2 Gerrit-Owner: Bankim Bhavsar <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Bankim Bhavsar <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 06 Apr 2021 00:02:38 +0000 Gerrit-HasComments: Yes
