Mike Percy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12212 )

Change subject: [scripts] Add initial test scripts for backup/restore testing
......................................................................


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/12212/1/src/kudu/scripts/backup-perf.py
File src/kudu/scripts/backup-perf.py:

http://gerrit.cloudera.org:8080/#/c/12212/1/src/kudu/scripts/backup-perf.py@71
PS1, Line 71:   Backported from Python 2.7 as it's implemented as pure python 
on stdlib.
> Not really understanding the justification. I thought this was so you could
Yeah this allows us to run this script on RHEL6.6


http://gerrit.cloudera.org:8080/#/c/12212/1/src/kudu/scripts/backup-perf.py@76
PS1, Line 76:   output, unused_err = process.communicate()
> Nit: Python convention is to use _ in place of a variable whose value you d
This is a straight backport / copy from build-support, copied so this script 
could be copied over to a host and used


http://gerrit.cloudera.org:8080/#/c/12212/1/src/kudu/scripts/backup-perf.py@244
PS1, Line 244:   # Actions
> I think you can simplify the true/false stuff with action='store_true'; doe
I tried that and found it confusing but maybe Grant knows this stuff better 
than me



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0c8efc3778b20687f0c1bc4c825f49f8f24e6d3b
Gerrit-Change-Number: 12212
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <granthe...@apache.org>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mpe...@apache.org>
Gerrit-Comment-Date: Fri, 11 Jan 2019 21:29:05 +0000
Gerrit-HasComments: Yes

Reply via email to