Lars Volker has posted comments on this change. Change subject: IMPALA-3581: Change location of minidump folders to log_dir ......................................................................
Patch Set 4: (6 comments) Thank you for the review. Please see PS5. http://gerrit.cloudera.org:8080/#/c/3171/4/bin/collect_minidumps.py File bin/collect_minidumps.py: Line 143: get_config_value > How about get config_parameter_value? Done Line 143: key > Can you rename this to something more descriptive? maybe config_parameter_n Done Line 163: '''Extracts the minidump directory path for a given role from the configuration file.''' > Update the comment, and briefly describe the logic. Maybe say if nothing is Done Line 164: result > it's confusing that you keep on reusing the same variable "result". Maybe r Done Line 167: result > Rename to minidump_path here as well Done Line 170: sys.exit(0) > Why isn't this an error any more? Done -- To view, visit http://gerrit.cloudera.org:8080/3171 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idecf5a314bfb8b0870e8aa4819c4fb39a107702f Gerrit-PatchSet: 4 Gerrit-Project: Impala Gerrit-Branch: cdh5-2.6.0_5.8.0 Gerrit-Owner: Lars Volker <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Lars Volker <[email protected]> Gerrit-Reviewer: Taras Bobrovytsky <[email protected]> Gerrit-HasComments: Yes
