From 24f9a25f549c21e2bc0297f6817b97b4ea714b0b Mon Sep 17 00:00:00 2001 From: Ferry Boender Date: Thu, 18 Jun 2015 08:12:21 +0200 Subject: [PATCH] pyflake and pylint cleanups. --- src/daemon.py | 48 ++++++++--- src/formconfig.py | 49 +++++++---- src/formdefinition.py | 65 ++++++++++++--- src/formrender.py | 185 ++++++++++++++++++++++++++++++++---------- src/scriptform.py | 49 +++++++---- src/webapp.py | 180 +++++++++++++++++++++++++--------------- 6 files changed, 413 insertions(+), 163 deletions(-) diff --git a/src/daemon.py b/src/daemon.py index 4e92ff8..6a42b19 100644 --- a/src/daemon.py +++ b/src/daemon.py @@ -1,3 +1,7 @@ +""" +Provide daemon capabilities via the Daemon class. +""" + import logging import os import sys @@ -8,10 +12,13 @@ import atexit class DaemonError(Exception): + """ + Default error for Daemon class. + """ pass -class Daemon: # pragma: no cover +class Daemon(object): # pragma: no cover """ Daemonize the current process (detach it from the console). """ @@ -27,17 +34,24 @@ class Daemon: # pragma: no cover self.log_file = log_file self.foreground = foreground + log_fmt = '%(asctime)s:%(name)s:%(levelname)s:%(message)s' logging.basicConfig(level=log_level, - format='%(asctime)s:%(name)s:%(levelname)s:%(message)s', + format=log_fmt, filename=self.log_file, filemode='a') self.log = logging.getLogger('DAEMON') - self.shutdown_cb = None + self.shutdown_callback = None - def register_shutdown_cb(self, cb): - self.shutdown_cb = cb + def register_shutdown_callback(self, callback): + """ + Register a callback to be executed when the daemon is stopped. + """ + self.shutdown_callback = callback def start(self): + """ + Start the daemon. Raises a DaemonError if it's already running. + """ self.log.info("Starting") if self.is_running(): self.log.error('Already running') @@ -46,6 +60,10 @@ class Daemon: # pragma: no cover self._fork() def stop(self): + """ + Stop the daemon. Raises a DaemonError if the daemon is ot running, + which is determined by examaning the PID file. + """ if not self.is_running(): raise DaemonError("Not running") @@ -101,6 +119,11 @@ class Daemon: # pragma: no cover return True def _fork(self): + """ + Fork the current process daemon-style. Forks twice, closes file + descriptors, etc. A signal handler is also registered to be called if + the daemon received a SIGTERM signal. + """ # Fork a child and end the parent (detach from parent) pid = os.fork() if pid > 0: @@ -114,9 +137,9 @@ class Daemon: # pragma: no cover pid = os.fork() if pid > 0: self.log.info("PID = {0}".format(pid)) - f = file(self.pid_file, 'w') - f.write(str(pid)) - f.close() + pidfile = file(self.pid_file, 'w') + pidfile.write(str(pid)) + pidfile.close() sys.exit(0) # End parent atexit.register(self._cleanup) @@ -124,9 +147,9 @@ class Daemon: # pragma: no cover # Close STDIN, STDOUT and STDERR so we don't tie up the controlling # terminal - for fd in (0, 1, 2): + for fdescriptor in (0, 1, 2): try: - os.close(fd) + os.close(fdescriptor) except OSError: pass @@ -139,7 +162,10 @@ class Daemon: # pragma: no cover return pid def _cleanup(self, sig=None): + """ + Remvoe pid files and call registered shutodnw callbacks. + """ self.log.info("Received signal {0}".format(sig)) if os.path.exists(self.pid_file): os.unlink(self.pid_file) - self.shutdown_cb() + self.shutdown_callback() diff --git a/src/formconfig.py b/src/formconfig.py index 251b3d8..4eadea7 100644 --- a/src/formconfig.py +++ b/src/formconfig.py @@ -1,3 +1,9 @@ +""" +FormConfig is the in-memory representation of a form configuration JSON file. +It holds information (title, users, the form definitions) on the form +configuration being served by this instance of ScriptForm. +""" + import logging import stat import os @@ -5,16 +11,20 @@ import subprocess class FormConfigError(Exception): + """ + Default error for FormConfig errors + """ pass -class FormConfig: +class FormConfig(object): """ FormConfig is the in-memory representation of a form configuration JSON file. It holds information (title, users, the form definitions) on the form configuration being served by this instance of ScriptForm. """ - def __init__(self, title, forms, users=None, static_dir=None, custom_css=None): + def __init__(self, title, forms, users=None, static_dir=None, + custom_css=None): self.title = title self.users = {} if users is not None: @@ -27,7 +37,8 @@ class FormConfig: # Validate scripts for form_def in self.forms: if not stat.S_IXUSR & os.stat(form_def.script)[stat.ST_MODE]: - raise FormConfigError("{0} is not executable".format(form_def.script)) + msg = "{0} is not executable".format(form_def.script) + raise FormConfigError(msg) def get_form_def(self, form_name): """ @@ -70,29 +81,33 @@ class FormConfig: # Validate params if form.output == 'raw' and (stdout is None or stderr is None): - raise ValueError('stdout and stderr cannot be None if script output is \'raw\'') + msg = 'stdout and stderr cannot be none if script output ' \ + 'is \'raw\'' + raise ValueError(msg) # Pass form values to the script through the environment as strings. env = os.environ.copy() - for k, v in form_values.items(): - env[k] = str(v) + for key, value in form_values.items(): + env[key] = str(value) # If the form output type is 'raw', we directly stream the output to # the browser. Otherwise we store it for later displaying. if form.output == 'raw': - p = subprocess.Popen(form.script, shell=True, - stdout=stdout, - stderr=stderr, - env=env) - stdout, stderr = p.communicate(input) - return p.returncode + proc = subprocess.Popen(form.script, shell=True, + stdout=stdout, + stderr=stderr, + env=env) + stdout, stderr = proc.communicate(input) + return proc.returncode else: - p = subprocess.Popen(form.script, shell=True, stdin=subprocess.PIPE, - stdout=subprocess.PIPE, stderr=subprocess.PIPE, - env=env) - stdout, stderr = p.communicate() + proc = subprocess.Popen(form.script, shell=True, + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + env=env) + stdout, stderr = proc.communicate() return { 'stdout': stdout, 'stderr': stderr, - 'exitcode': p.returncode + 'exitcode': proc.returncode } diff --git a/src/formdefinition.py b/src/formdefinition.py index c1ceaed..fe7273d 100644 --- a/src/formdefinition.py +++ b/src/formdefinition.py @@ -1,12 +1,18 @@ +""" +FormDefinition holds information about a single form and provides methods for +validation of the form values. +""" + import os import datetime class ValidationError(Exception): + """Default exception for Validation errors""" pass -class FormDefinition: +class FormDefinition(object): """ FormDefinition holds information about a single form and provides methods for validation of the form values. @@ -25,6 +31,9 @@ class FormDefinition: self.allowed_users = allowed_users def get_field_def(self, field_name): + """ + Return the field definition for `field_name`. + """ for field in self.fields: if field['name'] == field_name: return field @@ -40,9 +49,11 @@ class FormDefinition: # First make sure all required fields are there for field in self.fields: - if 'required' in field and \ - field['required'] is True and \ - (field['name'] not in form_values or form_values[field['name']] == ''): + field_required = ('required' in field and + field['required'] is True) + field_missing = (field['name'] not in form_values or + form_values[field['name']] == '') + if field_required and field_missing: errors.setdefault(field['name'], []).append( "This field is required" ) @@ -51,14 +62,15 @@ class FormDefinition: for field in self.fields: field_name = field['name'] if field_name in errors: - # Skip fields that are required but missing, since they can't be validated + # Skip fields that are required but missing, since they can't + # be validated continue try: - v = self._field_validate(field_name, form_values) - if v is not None: - values[field_name] = v - except ValidationError, e: - errors.setdefault(field_name, []).append(str(e)) + value = self._field_validate(field_name, form_values) + if value is not None: + values[field_name] = value + except ValidationError, err: + errors.setdefault(field_name, []).append(str(err)) return (errors, values) @@ -75,6 +87,9 @@ class FormDefinition: return validate_cb(field_def, form_values) def validate_string(self, field_def, form_values): + """ + Validate a form field of type 'string'. + """ value = form_values[field_def['name']] maxlen = field_def.get('maxlen', None) minlen = field_def.get('minlen', None) @@ -87,6 +102,9 @@ class FormDefinition: return value def validate_integer(self, field_def, form_values): + """ + Validate a form field of type 'integer'. + """ value = form_values[field_def['name']] maxval = field_def.get('max', None) minval = field_def.get('min', None) @@ -104,6 +122,9 @@ class FormDefinition: return int(value) def validate_float(self, field_def, form_values): + """ + Validate a form field of type 'float'. + """ value = form_values[field_def['name']] maxval = field_def.get('max', None) minval = field_def.get('min', None) @@ -121,6 +142,9 @@ class FormDefinition: return float(value) def validate_date(self, field_def, form_values): + """ + Validate a form field of type 'date'. + """ value = form_values[field_def['name']] maxval = field_def.get('max', None) minval = field_def.get('min', None) @@ -140,6 +164,9 @@ class FormDefinition: return value def validate_radio(self, field_def, form_values): + """ + Validate a form field of type 'radio'. + """ value = form_values[field_def['name']] if not value in [o[0] for o in field_def['options']]: raise ValidationError( @@ -147,6 +174,9 @@ class FormDefinition: return value def validate_select(self, field_def, form_values): + """ + Validate a form field of type 'select'. + """ value = form_values[field_def['name']] if not value in [o[0] for o in field_def['options']]: raise ValidationError( @@ -154,6 +184,9 @@ class FormDefinition: return value def validate_checkbox(self, field_def, form_values): + """ + Validate a form field of type 'checkbox'. + """ value = form_values.get(field_def['name'], 'off') if not value in ['on', 'off']: raise ValidationError( @@ -161,6 +194,9 @@ class FormDefinition: return value def validate_text(self, field_def, form_values): + """ + Validate a form field of type 'text'. + """ value = form_values[field_def['name']] minlen = field_def.get('minlen', None) maxlen = field_def.get('maxlen', None) @@ -174,6 +210,9 @@ class FormDefinition: return value def validate_password(self, field_def, form_values): + """ + Validate a form field of type 'password'. + """ value = form_values[field_def['name']] minlen = field_def.get('minlen', None) @@ -183,6 +222,9 @@ class FormDefinition: return value def validate_file(self, field_def, form_values): + """ + Validate a form field of type 'file'. + """ value = form_values[field_def['name']] field_name = field_def['name'] upload_fname = form_values[u'{0}__name'.format(field_name)] @@ -190,6 +232,7 @@ class FormDefinition: extensions = field_def.get('extensions', None) if extensions is not None and upload_fname_ext not in extensions: - raise ValidationError("Only file types allowed: {0}".format(u','.join(extensions))) + msg = "Only file types allowed: {0}".format(u','.join(extensions)) + raise ValidationError(msg) return value diff --git a/src/formrender.py b/src/formrender.py index 60cc8b9..8b34fb4 100644 --- a/src/formrender.py +++ b/src/formrender.py @@ -1,37 +1,74 @@ -html_field = u''' +""" +FormRender takes care of the rendering of forms to HTML. +""" + +HTML_FIELD = u'''

  • {title}

    -

    {h_input} {errors}

    +

    + {h_input} + {errors} +

    • ''' -html_field_checkbox = u''' +HTML_FIELD_CHECKBOX = u'''
  • -

    {h_input}

    {title}

    {errors}

    +

    + {h_input} +

    {title}

    + {errors} +

    • ''' -class FormRender(): +class FormRender(object): + """ + FormRender takes care of the rendering of forms to HTML. + """ field_tpl = { - "string": u'', - "number": u'', - "integer": u'', - "float": u'', - "date": u'', - "file": u'', - "password": u'', - "text": u'', - "radio_option": u'{label}
    ', - "select_option": u'', - "select": u'', - "checkbox": u'', + "string": u'', + "number": u'', + "integer": u'', + "float": u'', + "date": u'', + "file": u'', + "password": u'', + "text": u'', + "radio_option": u'{label}
    ', + "select_option": u'', + "select": u'', + "checkbox": u'', } def __init__(self, form_def): self.form_def = form_def def cast_params(self, params): + """ + Casts values in `params` dictionary to the correct types and values for + use in the form rendering. + """ new_params = params.copy() if 'required' in new_params: @@ -52,60 +89,105 @@ class FormRender(): return new_params def r_field(self, field_type, **kwargs): + """ + Render a generic field to HTML. + """ params = self.cast_params(kwargs) method_name = 'r_field_{0}'.format(field_type) method = getattr(self, method_name, None) return method(**params) - def r_field_string(self, name, value, size=50, required=False, classes=None, style=""): + def r_field_string(self, name, value, size=50, required=False, + classes=None, style=""): + """ + Render a string field to HTML. + """ if classes is None: classes = [] tpl = self.field_tpl['string'] - return tpl.format(name=name, value=value, size=size, required=required, classes=classes, style=style) - - def r_field_number(self, name, value, minval=None, maxval=None, required=False, classes=None, style=""): + return tpl.format(name=name, value=value, size=size, required=required, + classes=classes, style=style) + + def r_field_number(self, name, value, minval=None, maxval=None, + required=False, classes=None, style=""): + """ + Render a number field to HTML. + """ if classes is None: classes = [] tpl = self.field_tpl['number'] - return tpl.format(name=name, value=value, minval=minval, maxval=maxval, required=required, classes=classes, style=style) - - def r_field_integer(self, name, value, minval=None, maxval=None, required=False, classes=None, style=""): + return tpl.format(name=name, value=value, minval=minval, maxval=maxval, + required=required, classes=classes, style=style) + + def r_field_integer(self, name, value, minval=None, maxval=None, + required=False, classes=None, style=""): + """ + Render a integer field to HTML. + """ if classes is None: classes = [] tpl = self.field_tpl['integer'] - return tpl.format(name=name, value=value, minval=minval, maxval=maxval, required=required, classes=classes, style=style) - - def r_field_float(self, name, value, minval=None, maxval=None, required=False, classes=None, style=""): + return tpl.format(name=name, value=value, minval=minval, maxval=maxval, + required=required, classes=classes, style=style) + + def r_field_float(self, name, value, minval=None, maxval=None, + required=False, classes=None, style=""): + """ + Render a float field to HTML. + """ if classes is None: classes = [] tpl = self.field_tpl['integer'] - return tpl.format(name=name, value=value, minval=minval, maxval=maxval, required=required, classes=classes, style=style) - - def r_field_date(self, name, value, required=False, classes=None, style=""): + return tpl.format(name=name, value=value, minval=minval, maxval=maxval, + required=required, classes=classes, style=style) + + def r_field_date(self, name, value, required=False, classes=None, + style=""): + """ + Render a date field to HTML. + """ if classes is None: classes = [] tpl = self.field_tpl['date'] - return tpl.format(name=name, value=value, required=required, classes=classes, style=style) + return tpl.format(name=name, value=value, required=required, + classes=classes, style=style) def r_field_file(self, name, required=False, classes=None, style=""): + """ + Render a file field to HTML. + """ if classes is None: classes = [] tpl = self.field_tpl['file'] - return tpl.format(name=name, required=required, classes=classes, style=style) - - def r_field_password(self, name, value, minval=None, required=False, classes=None, style=""): + return tpl.format(name=name, required=required, classes=classes, + style=style) + + def r_field_password(self, name, value, minval=None, required=False, + classes=None, style=""): + """ + Render a password field to HTML. + """ if classes is None: classes = [] tpl = self.field_tpl['password'] - return tpl.format(name=name, value=value, minval=minval, required=required, classes=classes, style=style) - - def r_field_text(self, name, value, rows=4, cols=80, required=False, classes=None, style=""): + return tpl.format(name=name, value=value, minval=minval, + required=required, classes=classes, style=style) + + def r_field_text(self, name, value, rows=4, cols=80, required=False, + classes=None, style=""): + """ + Render a text field to HTML. + """ if classes is None: classes = [] tpl = self.field_tpl['text'] - return tpl.format(name=name, value=value, rows=rows, cols=cols, required=required, classes=classes, style=style) + return tpl.format(name=name, value=value, rows=rows, cols=cols, + required=required, classes=classes, style=style) def r_field_radio(self, name, value, options, classes=None, style=""): + """ + Render a radio field to HTML. + """ if classes is None: classes = [] tpl_option = self.field_tpl['radio_option'] @@ -114,16 +196,25 @@ class FormRender(): checked = '' if o_value == value: checked = 'checked' - radio_elems.append(tpl_option.format(name=name, value=value, checked=checked, label=o_label, classes=classes, style=style)) + radio_elems.append(tpl_option.format(name=name, value=value, + checked=checked, label=o_label, classes=classes, + style=style)) return u''.join(radio_elems) def r_field_checkbox(self, name, checked, classes='', style=""): + """ + Render a checkbox field to HTML. + """ if classes is None: classes = [] tpl = self.field_tpl['checkbox'] - return tpl.format(name=name, checked=checked, classes=classes, style=style) + return tpl.format(name=name, checked=checked, classes=classes, + style=style) def r_field_select(self, name, value, options, classes=None, style=""): + """ + Render a select field to HTML. + """ if classes is None: classes = [] tpl_option = self.field_tpl['select_option'] @@ -132,16 +223,22 @@ class FormRender(): selected = '' if o_value == value: selected = 'selected' - select_elems.append(tpl_option.format(value=o_value, selected=selected, label=o_label, style=style)) + select_elems.append(tpl_option.format(value=o_value, + selected=selected, label=o_label, + style=style)) tpl = self.field_tpl['select'] - return tpl.format(name=name, select_elems=''.join(select_elems), classes=classes, style=style) + return tpl.format(name=name, select_elems=''.join(select_elems), + classes=classes, style=style) def r_form_line(self, field_type, title, h_input, classes, errors): + """ + Render a line (label + input) to HTML. + """ if field_type == 'checkbox': - html = html_field_checkbox + html = HTML_FIELD_CHECKBOX else: - html = html_field + html = HTML_FIELD return (html.format(classes=' '.join(classes), title=title, diff --git a/src/scriptform.py b/src/scriptform.py index 00d0b2c..8617ba9 100755 --- a/src/scriptform.py +++ b/src/scriptform.py @@ -1,6 +1,6 @@ #!/usr/bin/env python # -*- coding: utf-8 -*- - +# # Scriptform roughly works like this: # # 1. Instantiate a ScriptForm class. This takes care of loading the form config @@ -18,9 +18,9 @@ # 5. Depending on the request, a method is called on ScriptFormWebApp. These # methods render HTML to as a response. # 6. If a form is submitted, its fields are validated and the script callback -# is called. Depending on the output type, the output of the script is either -# captured and displayed as HTML to the user or directly streamed to the -# browser. +# is called. Depending on the output type, the output of the script is +# either captured and displayed as HTML to the user or directly streamed to +# the browser. # 7. GOTO 4. # 8. Upon receiving an OS signal (kill, etc) the daemon calls the shutdown # callback. @@ -28,6 +28,10 @@ # until the next request) to stop the server. # 10. The program exits. +""" +Main ScriptForm program +""" + import sys import optparse import os @@ -43,10 +47,13 @@ from webapp import ThreadedHTTPServer, ScriptFormWebApp class ScriptFormError(Exception): + """ + Default exception thrown by ScriptForm errors. + """ pass -class ScriptForm: +class ScriptForm(object): """ 'Main' class that orchestrates parsing the Form configurations and running the webserver. @@ -60,7 +67,8 @@ class ScriptForm: self.running = False self.httpd = None - self.get_form_config() # Init form config so it can raise errors about problems. + # Init form config so it can raise errors about kproblems. + self.get_form_config() def get_form_config(self): """ @@ -122,7 +130,8 @@ class ScriptForm: is called or something like SystemExit is raised in a handler. """ ScriptFormWebApp.scriptform = self - self.httpd = ThreadedHTTPServer((listen_addr, listen_port), ScriptFormWebApp) + self.httpd = ThreadedHTTPServer((listen_addr, listen_port), + ScriptFormWebApp) self.httpd.daemon_threads = True self.log.info("Listening on {0}:{1}".format(listen_addr, listen_port)) self.running = True @@ -136,10 +145,14 @@ class ScriptForm: """ self.log.info("Attempting server shutdown") - def t_shutdown(sf): - sf.log.info(self.websrv) - sf.httpd.socket.close() # Undocumented requirement to shut the server - sf.httpd.shutdown() + def t_shutdown(scriptform_instance): + """ + Callback for when the server is shutdown. + """ + scriptform_instance.log.info(self.websrv) + # Undocumented feature to shutdow the server. + scriptform_instance.httpd.socket.close() + scriptform_instance.httpd.shutdown() # We need to spawn a new thread in which the server is shut down, # because doing it from the main thread blocks, since the server is @@ -148,6 +161,9 @@ class ScriptForm: def main(): # pragma: no cover + """ + main method + """ usage = [ sys.argv[0] + " [option] (--start|--stop) ", " " + sys.argv[0] + " --generate-pw", @@ -205,15 +221,16 @@ def main(): # pragma: no cover log = logging.getLogger('MAIN') try: if options.action_start: - sf = ScriptForm(args[0], cache=not options.reload) - daemon.register_shutdown_cb(sf.shutdown) + cache = not options.reload + scriptform_instance = ScriptForm(args[0], cache=cache) + daemon.register_shutdown_callback(scriptform_instance.shutdown) daemon.start() - sf.run(listen_port=options.port) + scriptform_instance.run(listen_port=options.port) elif options.action_stop: daemon.stop() sys.exit(0) - except Exception, e: - log.exception(e) + except Exception, err: + log.exception(err) raise if __name__ == "__main__": # pragma: no cover diff --git a/src/webapp.py b/src/webapp.py index 47a2336..eb850b9 100644 --- a/src/webapp.py +++ b/src/webapp.py @@ -1,3 +1,8 @@ +""" +The webapp part of Scriptform, which takes care of serving requests and +handling them. +""" + from SocketServer import ThreadingMixIn import BaseHTTPServer from BaseHTTPServer import BaseHTTPRequestHandler @@ -12,7 +17,7 @@ import hashlib from formrender import FormRender -html_header = u''' +HTML_HEADER = u'''