4
\$\begingroup\$

Here's a very small and basic script that sends commands over SSH to another computer on my network:

#!/bin/python
import sys, os
commands = ""
for i in sys.argv[1:]:
        commands += " " + i;

if len(sys.argv) <= 1:
        os.system("sshpass -p VerySecrectPassword ssh pi@172.16.0.141")
else:
        os.system("sshpass -p VerySecrectPassword ssh pi@172.16.0.141" + commands)

Am I being very "Pythonic"? Is it better to use a for loop to join all the commands or the join function?

commands = " ".join(sys.argv[1:])

Personally, I think the for loop reads better, but that's just my opinion and I'm a newbie. Which is considered the better way to do things?

Furthermore:

  • Are there any other areas in this script that could be improved?
  • Is it a bad idea to store the password in the script (located at /usr/bin)?

Note: This script is stored on a computer where I am the only user (except root).

\$\endgroup\$
8
\$\begingroup\$

To answer your questions: Yes.

Your code could be boiled down to:

#!/bin/python
import sys
import os

os.system("sshpass -p VerySecrectPassword ssh pi@172.16.0.141" + " " .join(sys.argv[1:]))

Note that I also put the imports on two lines, as recommended by PEP8, Python's official style-guide.

But I would actually re-expand it to:

#!/bin/python
import sys
import os

data = {"user": "pi",
        "host": "172.16.0.141",
        "password": "VerySecret",
        "commands": " " .join(sys.argv[1:])}

command = "sshpass -p {password} ssh {user}@{host} {commands}"
os.system(command.format(**data))

This gives you some easier ways to modify the configuration. It would also be easier to implement reading this configuration from a file now.

Note that bash (or whatever shell you use) will not care about the trailing whitespace in case commands is the empty string.

The next step would be using a ssh library, like @Simon recommended in his comment.

One way to do it would be first setting up password-less login and then using this answer and a comment within it:

#!/bin/python
import sys
import paramiko

rpi = {"username": "pi",
       "hostname": "172.16.0.141"}
command = " " .join(sys.argv[1:])}

ssh = paramiko.SSHClient()
ssh.load_system_host_keys()
ssh.set_missing_host_key_policy(paramiko.AutoAddPolicy())
ssh.connect(**rpi)
ssh_stdin, ssh_stdout, ssh_stderr = ssh.exec_command(command)

Here the ssh.set_missing_host_key_policy(paramiko.AutoAddPolicy()) is needed because paramiko does not read the hosts file correctly.

\$\endgroup\$
  • \$\begingroup\$ you could also take the password from an environment variable, which is probably better than having it in the source code \$\endgroup\$ – DanJ Oct 8 '16 at 17:31
  • \$\begingroup\$ Does that work out of the box or do you have to set up the config file? \$\endgroup\$ – Simon Oct 9 '16 at 14:37
  • \$\begingroup\$ Nice. Thought it just loaded the optional config file att first i.e. .ssh/config \$\endgroup\$ – Simon Oct 9 '16 at 16:51

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.