4
\$\begingroup\$

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 content of catalina.out with less'), action='store_true')
 parser.add_argument('-L', '--acclog', help=('Show localhost_access_log.(date).txt output on screen'), action='store_true')
 parser.add_argument('-q', '--manalog', help=('Display logs/app-manager.log with less'), action='store_true')
 parser.add_argument('-C', '--runconfig', help=('Run Tomcat reconfiguration process - creates server.xml, some symlinks etc'), action='store_true')
 parser.add_argument('-u', '--selfupgrade', help=('Download archive with app-manager directory from Nexus, delete current one, and unpack new fiels'), action='store_true')
 parser.add_argument('-d', '--deployapp', help=('Download archive with given APP version in to app-application directory'), action='store_true')
 parser.add_argument('-e', '--encryptconf', help=('Encrypting configuration files in conf/catalina directory'), action='store_true')
 parser.add_argument('-b', '--decryptconf', help=('Decrypting configuration files in conf/catalina directory'), action='store_true')
 parser.add_argument('-E', '--encryptpwd', help=('Encrypting input passwords and return hashed password'), action='store_true')
 parser.add_argument('-D', '--checkdbcon', help=('Check connections to all databases in external.properties'), action='store_true')
 parser.add_argument('-Q', '--scp', help=('Copy specified file to main host'), action='store_true')
if len(sys.argv) <= 1:
 parser.print_help()
 sys.exit(1)
else:
 return(parser.parse_args())
def runaction(ENV):
 # will select each option = True
 # import necessary class fil from 'lib'
 # and call appropriate method
 if options_set.start:
 from lib import appmanage_functions
 print('Start Tomcat')
 '''appmanage_functions.AppManage - contains all application commands - start, stop, status, etc'''
 a = appmanage_functions.AppManage(ENV)
 a.start(ENV)
 if options_set.status:
 from lib import appmanage_functions
 print('Tomcat status')
 appmanage_functions.status(main_functions.CATALINA_PID)
 [other options below]
if __name__ == '__main__':
 env = os.environ['ENV']
 options_set = getopts()
 main_f = main_functions.Main(env)
 main_f.act_opts(options_set)
 print('\nAPPmanager started at %s ' % (time.strftime('%d, %b %Y at %H:%M:%S')))
 print('Working on %s environment with APP_BASEDIR=%s.' % (env, main_f.APP_BASEDIR))
 print('Running with options: %s \n' % main_f.opts2)
 runaction(env)

File main_functions, with class Main will be parent for all other classes, to pass "global variables" to them:

import os, sys, platform
class Main:
 def __init__ (self, env):
 '''Will set APP_BASEDIR - main APP variable, pointed to $HOME/APP directory.
 Also - will set $smem and $xmem settings for CATALINA_OPTS'''
 print('ENV: %s\n' % env)
 list = ('SIT', 'DEV', 'UAT', 'REG')
 try:
 self.HOME = os.environ['HOME']
 if env == 'PROD':
 self.APP_BASEDIR = '/usr/local/app_install/APP'
 self.smem = '64G'
 self.xmem = '64G'
 else:
 self.APP_BASEDIR = os.path.join(self.HOME, 'APP')
 if env in list:
 self.smem = '32G'
 self.xmem = '32G'
 else:
 self.smem = '2G'
 self.xmem = '2G'
 except KeyError as e:
 print('Can\'t find HOME variable! Exit: %s' % e)
 def setvars(self, env):
 APP_INSTALL = os.path.join(self.APP_BASEDIR, 'app-manager')
 self.APP_APP = os.path.join(self.APP_BASEDIR, 'app-application')
 self.APP_CONFDIR = os.path.join(APP_INSTALL, 'conf')
 self.APP_CONFIGFILE = os.path.join(self.APP_CONFDIR, 'server', 'server.xml')
 APP_ENCRYPTION_PASSWORD = 'secret'
 APP_MARKER = 'secret'
 os.environ['CATALINA_HOME'] = os.path.join(self.APP_BASEDIR, 'app-server')
 os.environ['BASEDIR'] = os.environ['CATALINA_HOME']
 APP_LIBS = os.path.join(self.APP_CONFDIR, 'lib')
 CATALINA_PID = os.path.join(os.environ['CATALINA_HOME'], 'conf', 'catalina.pid')
 os.environ['CATALINA_OUT'] = os.path.join(self.APP_BASEDIR, 'logs', 'catalina.out')
 self.LOGGING_CONFIG = os.path.join('-Djava.util.logging.config.file=' + self.APP_CONFDIR, 'server', 'logging.properties')
 LOG4J = os.path.join(self.APP_CONFDIR, 'log4j', 'log4j.' + env + '.xml')
 os.environ['LOG4J'] = LOG4J
 if platform.system() == 'Windows':
 JAVA_HOME = os.path.join(self.APP_BASEDIR, 'Java', 'Windows', 'jre')
 os.environ['JAVA_HOME'] = JAVA_HOME
 os.environ['JRE_HOME'] = os.path.join(JAVA_HOME)
 os.environ['APP_SECURE_LOCATION_PWD'] = os.path.join(self.HOME, 'security', 'secret.key')
 os.environ['APP_EXT_PROP'] = os.path.join(self.HOME, 'security', 'external.properties')
 elif platform.system() == 'Linux':
 JAVA_HOME = os.path.join(self.APP_BASEDIR, 'Java', 'Linux')
 else:
 print('Unknow OS type, exit.')
 sys.exit(1)
 JRE_HOME = os.path.join(JAVA_HOME, 'jre')
 print('JAVA_HOME = %s' % (os.environ['JAVA_HOME']))
 print('JRE_HOME = %s' % JRE_HOME)
 if env == 'PROD':
 self.OUTPUT_DIR = os.path.join('var', 'app_logs' + os.environ['hostname'])
 else:
 self.OUTPUT_DIR = os.path.join(self.APP_BASEDIR, 'logs')
 print('app_version_file = %s' % (os.path.join(self.APP_CONFDIR, 'app.version')))
 with open(os.path.join(self.APP_CONFDIR, 'app.version'), 'r') as app_version_file:
 self.APP_VERSION = app_version_file.readline()
 print('APP_VERSION: %s' % self.APP_VERSION)
 def act_opts(self, RES):
 self.opts = vars(RES)
 self.opts2 = {}
 for key in self.opts.keys():
 if self.opts[key]:
 self.opts2[key] = self.opts[key]

And file application_functions.py to manage application:

from main_functions import Main
import os, subprocess
class AppManage(Main):
 ...
 def start(self, env):
 self.setvars(env)
 os.environ['CATALINA_OPTS'] = '-d64 -server -Xms%s -Xmx%s\
 -XX:+UseCodeCacheFlushing -XX:ReservedCodeCacheSize=64M\
 -XX:ReservedCodeCacheSize=64M -XX:MaxPermSize=2048M\
 -Djavax.net.debug=ssl,handshake\
 -Denv=%s\
 -Doutput.dir=%s\
 -Dapp.app=%s\
 -Dapp.version=%s\
 -Dapp.conf=%s' % (self.smem, self.xmem, env, self.OUTPUT_DIR, self.APP_APP, self.APP_VERSION.rstrip(), self.APP_CONFDIR)
 # will fix here for Linux/Windows
 cata = os.path.join(os.environ['CATALINA_HOME'], 'bin', 'catalina.bat')
 subprocess.call(("%s start -config %s" % (cata, self.APP_CONFIGFILE)), shell=True)
asked Feb 23, 2015 at 12:34
\$\endgroup\$
7
  • \$\begingroup\$ Could you include the rest of the code, or at the very least, include the functions as a whole? Right now it's very fragmented code and I worry that answers may not be able to for see all side effects. \$\endgroup\$ Commented Feb 23, 2015 at 14:41
  • \$\begingroup\$ @Pimgd I don't sure... Some of them are really big, and have lot of "repeatable" parts (getopts() for example - all the same, but different options, or __init__ im Main - there is just lot of variables definitions). Is it have sense to past them too? Or - may be you could specify - what function need here to be in 'full view'? \$\endgroup\$ Commented Feb 23, 2015 at 14:48
  • 1
    \$\begingroup\$ Repeatable parts are something you will most definitely get an answer on which you can use to improve it, so why not post it? It's okay to leave functions out, but don't post half functions. Then it just becomes hard to see what the code does where. For you it's boilerplate, for us it's "Okay, so this class manages this, and that class manages that... ... here's where it configures the things..." It's like a guidepost, so to say. Right now there's 3 votes to close your question and it's likely it will get closed soon if the code isn't expanded. \$\endgroup\$ Commented Feb 23, 2015 at 14:56
  • 1
    \$\begingroup\$ @setevoy I know, but just make note of that in your question and you will probably get an answer on it. If you don't, you can comment on the answer and ask about it. \$\endgroup\$ Commented Feb 23, 2015 at 16:18
  • 1
    \$\begingroup\$ Have a look at this list of libraries for easy creation of CLI tools here. \$\endgroup\$ Commented Feb 23, 2015 at 16:29

1 Answer 1

2
\$\begingroup\$

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.

answered May 2, 2015 at 5:36
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.