Prev: Employee motivation in the software industry
Next: Pickling an extension type subclasses problems
From: Jean-Michel Pichavant on 5 Feb 2010 11:40
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

From: John Posner on 5 Feb 2010 11:51
On 2/5/2010 11:26 AM, Gerald Britton wrote:
> sure, but it will fit nicely on one line if you like
>
> On Fri, Feb 5, 2010 at 11:22 AM, John Posner<jjposner(a)optimum.net> wrote:
>
>> On 2/5/2010 11:06 AM, Gerald Britton wrote:
>>
>>> [snip]
>>>
>>>
>>>> Last August [1], I offered this alternative:
>>>>
>>>> self.cmd = (cmd.replace(r'${ADDR}',ip)
>>>> if isinstance(cmd, str) else
>>>> cmd)
>>>>
>>>> But it didn't get much love in this forum!
>>>>
>>> I'd probably go for that one as well though I might consider removing
>>> the outer parentheses.
>>>
>> Agreed ... except that you *need* the outer parentheses if the statement
>> occupies multiple source lines.
>>
>> -John
>>
>>
>>
>>
>
>
>
Did you mean to take this off-list? Also, I'm contractually obligated
to admonish you not to "top post".

At any rate, I proposed the 3-line format specifically because it
separates the data values from the if-then-else machinery, making it
easier (for me) to read. But there was considerable resistance to
spending so much vertical space in the source code.

-John

From: Gerald Britton on 5 Feb 2010 11:53
>
> Did you mean to take this off-list?

Nope -- just hit the wrong key

 Also, I'm contractually obligated to
> admonish you not to "top post".

Contract?

>
> At any rate, I proposed the 3-line format specifically because it separates
> the data values from the if-then-else machinery, making it easier (for me)
> to read. But there was considerable resistance to spending so much vertical
> space in the source code.

Weird! It's three lines and the original was four lines was it not>?

--
Gerald Britton
From: mk on 5 Feb 2010 12:07
Jean-Michel Pichavant wrote:

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

Understandable.

> 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 ?

You get this:

Linux RH [17:35] root ~ # cssh.py -c "ps|wc" -s /tmp/s.sh

Following options that you specified are mutually exclusive:
-s / --script (value: /tmp/s.sh)
-c / --cmd (value: ps|wc)
You have to specify exactly one of options -c / --cmd, or -s / --script,
or -y / --copy.

I wrote additional logic to handle such situations, I don't rely
entirely on optparse.

> 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'

True. But you have to copy the script to remote machine in the first
place. It's more convenient to do this using one option (copy to remote
machine & execute there).

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

Err but this is command to copy a local file/dir to *remote* machine.
Like scp (in fact it uses scp protocol internally).

>
> 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
>
>

From: mk on 5 Feb 2010 12:35
mk wrote:
>
> > 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 ?


Arrgh this is my bad day. You get this:

Linux RH [17:35] root ~ # cssh.py -i linux

You have to specify exactly one of the following:
- command to run remotely, using -c command / --cmd command, or
- script to run remotely, using -s scriptfile / --script scriptfile, or
- file/directory to copy, using -y file_or_dir / --copy file_or_dir

First  |  Prev  |  Next  |  Last
Pages: 1 2 3 4 5
Prev: Employee motivation in the software industry
Next: Pickling an extension type subclasses problems