HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Multipurpose command-line utility to manage a web application

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
lineapplicationutilitymanagewebmultipurposecommand

Problem

I'm creating a command-line tool which is supposed to work on both Windows and Linux OS. It will be used to manage our application - start, stop, deploy etc.

We have it in bash scripts now (and it works very well for about 6 month), but I want rewrite it in Python with classes.

It's big enough, so here are only a few main parts.

I'm first trying to use classes here. My idea is to have one Main class, which will be used by all other child classes to get necessary variables (does classes && inheritance intend at all for such "role"?).

Whole system based on two environment variables - ENV (environment, where tool started, depending on it - will use different setting files for application on so on), and HOME - path to where application files, manager-tool, Java, Tomcat etc placed.

I'm afraid classes here will only it more "spaghetti" with my hands. What am I doing wrong here, mainly about classes usage?

Main file - APPmanager.py contains:

```
#!/usr/bin/env python

import argparse, os, sys, time

# I have directory 'lib' with empty __init__.py there
# in this dir - all calsses files will be placed
from lib import main_functions

def getopts():

# totally - there is about 20 options

parser = argparse.ArgumentParser()

parser.add_argument('-s', '--start', help=('Start APP'), action='store_true')
parser.add_argument('-S', '--stop', help=('Stop APP'), action='store_true')
parser.add_argument('-F', '--stopforce', help=('Shutdown-force APP (30 sec wit, then kill -KILL)'), action='store_true')
parser.add_argument('-c', '--status', help=('Display APP status'), action='store_true')
parser.add_argument('-R', '--restart', help=('Restart APP'), action='store_true')

parser.add_argument('-v', '--version', help=('Set new APP VERSION'), metavar = 'BUILD NUMBER')

parser.add_argument('-t', '--talog', help=('Display content of app-app.log with tail'), action='store_true')
parser.add_argument('-l', '--catalog', help=('Display conten

Solution

I see several issues with the first script, APPmanager.py,
the others seem quite fine.

Imports

From PEP8:

  • Imports should usually be on separate lines: split import argparse, os, sys, time to multiple lines



  • Imports should be put at the top of the file: move from lib import appmanage_functions to the top, there's no good reason to do this import later



Don't execute code in global scope

It's good to make it possible to import your Python scripts as modules without executing anything.
This can be useful to reuse the code in other scripts,
and in unit testing too.
Code like this in the global scope makes that impossible:

if len(sys.argv) <= 1:
    parser.print_help()
    sys.exit(1)
else:
    return(parser.parse_args())


Move this snippet out of the global scope.

Also, you could simplify the condition to this:

if not sys.argv:


Move code out of global scope

Code in global scope guarded by if __name__ == '__main__' is ok,
but it's even better to move such code to a function,
for example called main.
The difference is that variables you define within the if __name__ == '__main__' may be visible in the functions you call,
which can lead to confusion and bugs.

Code Snippets

if len(sys.argv) <= 1:
    parser.print_help()
    sys.exit(1)
else:
    return(parser.parse_args())
if not sys.argv:

Context

StackExchange Code Review Q#82362, answer score: 2

Revisions (0)

No revisions yet.