My Pythonic take on Psexec

I've created a little program that basically does the same thing as psexec. It will connect to a host via computer name, or IP address, run the given command, and log the output into a file (little extra for myself). I would like some critique on what I've done, what can I do better, what did I do well, etc.. Some key points I would like to look at are (of course critique everything):

• Logging to a file, is there better ways to do so?

• Connecting and executing a shell command quicker

```
import wmi
import sys
import socket
import subprocess
import logging
import getpass
import os
import time
from colorlog import ColoredFormatter

log_level = logging.INFO
logger_format = "[%(log_color)s%(asctime)s %(levelname)s%(reset)s] %(log_color)s%(message)s%(reset)s"
logging.root.setLevel(log_level)
formatter = ColoredFormatter(logger_format, datefmt="%I:%M:%S")
stream = logging.StreamHandler()
stream.setLevel(log_level)
stream.setFormatter(formatter)
LOGGER = logging.getLogger('pyshellConfig')
LOGGER.setLevel(log_level)
LOGGER.addHandler(stream)

def create_log(dest_ip, host_ip, command, data):
"""
:param dest_ip: Destination IP address
:param host_ip: IP address of where the command was run
:param command: Command that was run
:param data: Output of the command

Write the output of the run command to a log file for further analysis
"""
log_path = "C:\\Users\\{}\\AppData\\pyshell".format(getpass.getuser())
if not os.path.isdir(log_path):
os.mkdir(log_path)

with open("{}\\{}_LOG.LOG".format(log_path, __file__), "a+") as log:
log.write("![{}][LOG FROM:{} TO:{}]COMMAND RUN: {} OUTPUT:{}\n".format(time.strftime("%m-%d-%Y::%H:%M:%S"),
host_ip, dest_ip,
command, data))

def connect(hostname, command=None):
"""
:param hostname: Computer name or IP address

Keep your imports grouped. From PEP 8.

Imports should be grouped in the following order:

• standard library imports

• related third party imports

• local application/library specific imports

Follow Python's EAFP principle instead of checking if something exists. Plus when it comes to folder creation it will also help you avoid race-condition.

import errno

try:
    os.mkdir(log_path)
except Exception as e:
    if e.errno != errno.EEXIST:
        # If error is something other than file
        # path already exists then handle it here.
        pass

Note my use of except Exception as e here instead of except Exception, e. The latter syntax is deprecated and has been removed in Python 3. Hence go for as based format.

Your try-except block is huge. Cover minimum statements at once, it will help you in narrowing down the actual problem easily and you will be able to show a much better error message to user. Have multiple try-excepts as long as they don't make the code too unreadable.

Instead of dealing with sys.argv you could use the arsparse module to handle command line args in much better way.

error_message = "" statement is not required. You can simply define it wherever you need it instead of defining a default value.

[1::] and [2::] can be simply written as [1:] and [2:] respectively.

The IndexError at the end is redundant because you're already checking for if not [1::] earlier and exiting the script right away, hence you would never hit that exception. Note that slicing never raises IndexError.

call.communicate("Running remote command...") returns both STDOUT and STDIN. You can consider using them separately.

Plus you could also check the status code by doing call.returncode. A non-zero exit code means an error.

log_path = "C:\\Users\\{}\\AppData\\pyshell".format(getpass.getuser())

A hard-coded path is not a good idea. Perhaps ask the user for a path?

You can also get path to user's home directory using os.path.expanduser.

log_path = r"{home}\AppData\pyshell".format(home=os.path.expanduser('~'))

Note that I also added r"" so that we can simply use a single backslash.

StackExchange Code Review Q#154197, answer score: 5