Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Issue 5611225295618048: Issue 1267 - Provide a way to run some command on multiple hosts (Closed)

Created:
Aug. 25, 2014, 2:43 p.m. by Wladimir Palant
Modified:
Sept. 3, 2014, 2:54 p.m.
Reviewers:
mathias
Visibility:
Public.

Description

This moves a bunch of code from kick.py into run.py, the only change was making resolveHostList call getValidHosts itself rather than getting the results as parameter. I also made the user parameter optional, as it is optional for the SSH command.

Patch Set 1 #

Total comments: 2

Patch Set 2 : Print all logging output to stderr #

Unified diffs Side-by-side diffs Delta from patch set Stats (+144 lines, -88 lines) Patch
M .hgignore View 1 1 chunk +1 line, -0 lines 0 comments Download
M kick.py View 1 2 chunks +8 lines, -88 lines 0 comments Download
A run.py View 1 1 chunk +135 lines, -0 lines 0 comments Download

Messages

Total messages: 6
Wladimir Palant
Aug. 25, 2014, 2:43 p.m. (2014-08-25 14:43:07 UTC) #1
mathias
Although the changes and purpose of this patch are pretty straight-forward, I'm not really convinced ...
Aug. 25, 2014, 4:21 p.m. (2014-08-25 16:21:25 UTC) #2
Wladimir Palant
Print all logging output to stderr
Aug. 29, 2014, 10:04 p.m. (2014-08-29 22:04:11 UTC) #3
Wladimir Palant
Print all logging output to stderr
Aug. 29, 2014, 10:08 p.m. (2014-08-29 22:08:39 UTC) #4
Wladimir Palant
The reason why it is two scripts is pretty simple: one is a general-purpose script ...
Aug. 29, 2014, 10:11 p.m. (2014-08-29 22:11:15 UTC) #5
mathias
Sept. 3, 2014, 1:17 p.m. (2014-09-03 13:17:17 UTC) #6
LGTM.

I'd still prefer a calling scheme like "./run.py $command" instead of two
different scripts, but that's basically just a personal preference.

On 2014/08/29 22:11:15, Wladimir Palant wrote:
> What's code duplication you see there? From all I can tell, it's only the
> parameter parsing - and even that isn't really the same.
> 
>
http://codereview.adblockplus.org/5611225295618048/diff/5629499534213120/run.py
> File run.py (right):
> 
>
http://codereview.adblockplus.org/5611225295618048/diff/5629499534213120/run....
> run.py:134: print 'Running on %s...' % host
> On 2014/08/25 16:21:25, matze wrote:
> > If the host information is required, one could prefix each line instead of
> > printing host notifications
> 
> I'm not usually using scripts on the output. And there might not even be any
> output - I simply want to see the current progress when running a command on
> twenty servers.
> 
> > one could prefix each line
> 
> It's not that easy IMHO. Piping command output in Python while the command is
> running requires a second thread - I'm not ready to go there for this simple
> tool. An external tool like awk could do it as well, this adds a dependency
> however. In the end, maybe I can simply pipe it into a second Python process -
> but that's something to consider when I actually need this functionality.
> 
> > This behavior could even be kept when e.g. deciding to implement
> parallelization
> 
> I don't think I want to go there just yet :)

Powered by Google App Engine
This is Rietveld