7

I have a simple script that checks various Linux processes and, finding one of them, logs a particular message ("particular" in terms of referencing the service's name).

My question: what is the proper, Pythonic way to have a multi-conditional function return a boolean and a string (for use in a printed message)?

Here's a stripped-down version of my current solution (using tuples):

import subprocess
import time

def _isProcessRunning(cmd):
    return int(
            subprocess.check_output(
                '{} | grep -v grep | wc -l'.format(cmd),
                shell=True
                )
            ) > 0

def processGroupFound():
    if _isProcessRunning('ps auwxx | grep duplicity'):
        return (True, 'Duplicity')
    elif _isProcessRunning('who | grep pts'):
        return (True, 'SSH')
    elif _isProcessRunning('netstat -na | grep ESTA | grep 5901'):
        return (True, 'VNC')
    else:
        return (False, '')

def worker():
    while True:
        process_found, service_string = processGroupFound()
        if process_found:
            print('{} found; skipping'.format(service_string))
        else:
            print('Continuing on')
        time.sleep(10)


if __name__ == "__main__":
    worker()

This works, but I care about doing it correctly (stylistically, in particular, but if you glean incorrect logic in this brief sample, please feel free to comment there, too. Appreciate your assistance!

ABach
  • 3,592
  • 4
  • 22
  • 31
  • 4
    This is fine. Perfectly idiomatic. I might omit the parens (`return True, 'Duplicity'`), but then again I might not in some cases. – Mad Physicist Dec 09 '16 at 21:27
  • 2
    Lots of other options here too http://stackoverflow.com/questions/354883/how-do-you-return-multiple-values-in-python – Alex G Rice Dec 09 '16 at 21:27
  • 1
    I wouldn't bother with the parentheses around the `return` statements, but otherwise, seems okay. I'm always suspicious of `shell=True`, and using external utilities for things Python can do (the `grep` and `wc` work), but that's separate from what you're asking about. – ShadowRanger Dec 09 '16 at 21:28
  • Thanks, all, for your comments (especially yours, @ShadowRanger – I'm going to use shell processes for the bare minimum, then Python for the bulk of the work). – ABach Dec 09 '16 at 22:36

2 Answers2

6

An empty string in Python is "falsey", so it's somewhat redundant to return (False, ''). I might do this instead:

def processGroupFound():
    if _isProcessRunning('ps auwxx | grep duplicity'):
        return 'Duplicity'
    elif _isProcessRunning('who | grep pts'):
        return 'SSH'
    elif _isProcessRunning('netstat -na | grep ESTA | grep 5901'):
        return 'VNC'
    else:
        return ''

def worker():
    while True:
        service_string = processGroupFound()
        if service_string:
            print('{} found; skipping'.format(service_string))
        else:
            print('Continuing on')
        time.sleep(10)

(See 4.1 Truth Value Testing)

Joe Carey
  • 76
  • 3
  • 1
    Testing the returned value is C-ish instead of pythonic, that's why I prefer the exception solution. – Gribouillis Dec 09 '16 at 21:50
  • Thanks for your answer! I had forgotten about the "falsey-ness" of empty strings. – ABach Dec 09 '16 at 22:36
  • The answer @Gribouillis gave wasn't there when I started writing mine. I agree that his is more pythonic - it gets points under "ask forgiveness, not permission" at the least. – Joe Carey Dec 10 '16 at 03:29
3

I think this would be pythonic too (but may be it's just me)

class NoRunningService(RuntimeError): pass

def findService():
    if _isProcessRunning('ps auwxx | grep duplicity'):
        return 'Duplicity'
    elif _isProcessRunning('who | grep pts'):
        return 'SSH'
    elif _isProcessRunning('netstat -na | grep ESTA | grep 5901'):
        return 'VNC'
    else:
        raise NoRunningService

def worker():
    while True:
        try:
            service_string = findService()
        except NoRunningService:
            print('Continuing on')
        else:
            print('{} found; skipping'.format(service_string))
        time.sleep(10)
Gribouillis
  • 2,027
  • 1
  • 6
  • 12
  • Thanks so much. Psychologically, I grapple with the idea of an "exception" as a "fatal error that needs correcting." I'll work on that. ;) – ABach Dec 09 '16 at 22:37
  • It's much more than that. When a function does not handle a case, raise an exception! – Gribouillis Dec 09 '16 at 23:02