Taras Bobrovytsky has posted comments on this change.

Change subject: Add Breakpad minidump collection script
......................................................................


Patch Set 2:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/2997/2/bin/collect_minidumps.py
File bin/collect_minidumps.py:

Line 82:      if self.start_time and self.start_time > time_modified:
       :         continue
> start_time is optional
Yeah, we are checking if start_time is not specified (None), it will be ignored.


Line 104: less
> (or equal to)
Done


Line 152:     if not os.path.isdir(result):
        :       msg = 'Error: minidump directory "{0}" does not exist (or is 
not a directory).'
        :       print >> sys.stderr, msg.format(result)
> I am not sure if this calls for an exception. One scenario in which this di
Done


Line 152:     if not os.path.isdir(result):
        :       msg = 'Error: minidump directory "{0}" does not exist (or is 
not a directory).'
        :       print >> sys.stderr, msg.format(result)
> The directories are created during daemon startup, so if that directory doe
Done


Line 157:   
> too many spaces
Done


Line 164: conf_dir
> Add help message.
Done


Line 165: role
> I think this should be renamed to role_type
Done


Line 173:  
> add a comma here
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/2997
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I85b3643133e28eca07507ac2a79acbf73128456f
Gerrit-PatchSet: 2
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Taras Bobrovytsky <[email protected]>
Gerrit-Reviewer: Ishaan Joshi <[email protected]>
Gerrit-Reviewer: Lars Volker <[email protected]>
Gerrit-Reviewer: Taras Bobrovytsky <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-HasComments: Yes

Reply via email to