which

[Jean-Michel Pichavant]

mk wrote:
> Jean-Michel Pichavant wrote:
>> If you can change your program interface, then do it, if not then 
>> you're right you don't have much choice as you are suffering from the 
>> program poor interface.
>> You can fix this problem by explicitly asking for the thing you want 
>> to do, instead of guessing by inspecting the argument nature.
>>
>> myProg --help
>>
>> usage : myProg command [args]
>>    command list:
>>       - cmd: execute the given <arg1> command line
>>       - exec: execute the given script file named <arg1>
>>       - copy: copy <arg1> to <arg2>
>>
>> example:
>>  >myProg cmd "echo that's cool"
>>  >myProg exec /etc/init.d/myDaemon
>>  >myProg copy /tmp /tmp2
>>
>
> I sure can change the interface since I'm the author of the entire 
> program. But I don't see how I can arrange program in a different way: 
> the program is supposed to be called with -c parameter (command to 
> run), -s script to run, or -y file_or_dir_to_copy.
>
> Then, I start instances of SSHThread class to do precisely that, 
> separately for each ip/hostname:
>
>
> class SSHThread(threading.Thread):
>     def __init__(self, lock, cmd, ip, username, sshprivkey=None, 
> passw=None, port=22, script=None, remotedir=None):
>
>         threading.Thread.__init__(self)
>
>         self.lock = lock
>         if isinstance(cmd, str):
>             self.cmd = cmd.replace(r'${ADDR}',ip)
>         else:
>             self.cmd = cmd
>         self.ip = ip
>         self.username = username
>         self.sshprivkey = sshprivkey
>         self.passw = passw
>         self.port = port
>         self.conobj = None
>         self.conerror = ''
>         self.msgstr = ''
>         self.confailed = True
>         if script:
>             self.setpathinfo(script, remotedir=remotedir)
>         self.sentbytes = 0
>         self.finished = False
>         self.abort = False
>
> It gets called like this:
>
> th = SSHThread(lock, opts.cmd, ip, username=username, 
> sshprivkey=opts.key, passw=passw, port=port, script=opts.script, 
> remotedir=opts.remotedir)
>
>
> ..where all the options are parsed by ConfigParser.OptionParser(). So 
> they are either strings, or Nones.
>
> So in this context this is fine. But I wanted to make the class more 
> robust. Perhaps I should do smth like this before setting self.cmd?
>
> assert isinstance(cmd, basestring) or cmd is None, "cmd should be 
> string or None"
>
> and then:
>
> if cmd:
>     self.cmd = cmd.replace..
>
>
> ?
>
> Entire source code is here:
>
> http://python.domeny.com/cssh.py
>
> regards,
> mk
>

To be honest I have not enough courrage to dive into yout 1000 lines of 
script :-)

What I can say however:

1/ your interface is somehow broken. You ask actions through options (-c 
-y -s), meaning one can possibly use all these 3 options  together. Your 
code won't handle it (you are using elif statements). What happens if I 
use no option at all ? As the the optparse doc states : if an option is 
not optional, then it is not an option (it's a positional argument).

2/ executing a script, or copying a directory are both commands as well.
myProg -s /tmp/myscript.sh
is nothing more than
myProg -c '/bin/sh myscript.sh'

myProg -y file1
is nothing more than
myProg -c 'cp file1 towhatever'

3/ check your user parameters before creating your SSHThread, and create 
your SSHThread with already validated parameters. You don't want to 
pollute you SSHThread code with irrelevant user error check.


my humble conclusion:

1/ rewrite your interface with
prog command args [options]

2/ Simplify your SSHThread by handling only  shell commands

3/ at the CLI level (right after parameter validation), mute you copy & 
script
command to a shell command and pass it to SSHThread.

Cheers,

JM