Taras Bobrovytsky has posted comments on this change. Change subject: IMPALA-3581: Change location of minidump folders to log_dir ......................................................................
Patch Set 4: (6 comments) http://gerrit.cloudera.org:8080/#/c/3171/4/bin/collect_minidumps.py File bin/collect_minidumps.py: Line 143: key Can you rename this to something more descriptive? maybe config_parameter_name? Line 143: get_config_value How about get config_parameter_value? 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 specified, we will assume the minidumps go to log_dir/minidumps, etc Line 164: result it's confusing that you keep on reusing the same variable "result". Maybe rename it to minidump_path here? Line 167: result Rename to minidump_path here as well Line 170: sys.exit(0) Why isn't this an error any more? -- 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
