From 549a27e66c14a211347ba7b2bc234997d9b672d5 Mon Sep 17 00:00:00 2001 From: Jim Paris Date: Mon, 3 Aug 2020 23:46:34 -0400 Subject: [PATCH] Add flake8 tests and clean up warnings --- Makefile | 15 +++++---- nilmrun/__init__.py | 2 -- nilmrun/processmanager.py | 71 ++++++++++++++++++++------------------- nilmrun/server.py | 65 ++++++++++++++++++----------------- setup.cfg | 8 +++++ 5 files changed, 88 insertions(+), 73 deletions(-) diff --git a/Makefile b/Makefile index b15c708..eea3ed1 100644 --- a/Makefile +++ b/Makefile @@ -1,9 +1,6 @@ # By default, run the tests. all: test -test2: - nilmrun/trainola.py data.js - version: python3 setup.py version @@ -23,8 +20,11 @@ develop: docs: make -C docs +ctrl: flake +flake: + flake8 nilmrun lint: - pylint3 --rcfile=.pylintrc nilmdb + pylint3 --rcfile=setup.cfg nilmrun test: ifneq ($(INSIDE_EMACS),) @@ -37,12 +37,13 @@ else endif clean:: + find . -name '*.pyc' -o -name '__pycache__' -print0 | xargs -0 rm -rf rm -f .coverage - find . -name '*pyc' | xargs rm -f - rm -rf nilmtools.egg-info/ build/ MANIFEST.in + rm -rf nilmrun.egg-info/ build/ make -C docs clean gitclean:: git clean -dXf -.PHONY: all version dist sdist install docs lint test clean gitclean +.PHONY: all version dist sdist install docs test +.PHONY: ctrl lint flake clean gitclean diff --git a/nilmrun/__init__.py b/nilmrun/__init__.py index 0e72cb5..e44e9d1 100644 --- a/nilmrun/__init__.py +++ b/nilmrun/__init__.py @@ -1,5 +1,3 @@ -import nilmrun.processmanager - from ._version import get_versions __version__ = get_versions()['version'] del get_versions diff --git a/nilmrun/processmanager.py b/nilmrun/processmanager.py index 2e4190a..2b4e629 100644 --- a/nilmrun/processmanager.py +++ b/nilmrun/processmanager.py @@ -1,7 +1,5 @@ #!/usr/bin/python -from nilmdb.utils.printf import * - import threading import subprocess import io @@ -15,16 +13,18 @@ import tempfile import atexit import shutil + class ProcessError(Exception): pass + class LogReceiver(object): """Spawn a thread that listens to a pipe for log messages, and stores them locally.""" def __init__(self, pipe): self.pipe = pipe self.log = io.BytesIO() - self.thread = threading.Thread(target = self.run) + self.thread = threading.Thread(target=self.run) self.thread.start() def run(self): @@ -41,9 +41,10 @@ class LogReceiver(object): def clear(self): self.log = io.BytesIO() + class Process(object): """Spawn and manage a subprocess, and capture its output.""" - def __init__(self, argv, tempfile = None): + def __init__(self, argv, tempfile=None): self.start_time = None # Use a pipe for communicating log data @@ -55,9 +56,9 @@ class Process(object): # Spawn the new process try: - self._process = subprocess.Popen(args = argv, stdin = nullfd, - stdout = wpipe, stderr = wpipe, - close_fds = True, cwd = "/tmp") + self._process = subprocess.Popen(args=argv, stdin=nullfd, + stdout=wpipe, stderr=wpipe, + close_fds=True, cwd="/tmp") except (OSError, TypeError) as e: raise ProcessError(str(e)) finally: @@ -69,7 +70,7 @@ class Process(object): self.start_time = time.time() self.pid = str(uuid.uuid1(self._process.pid or 0)) - def _join(self, timeout = 1.0): + def _join(self, timeout=1.0): start = time.time() while True: if self._process.poll() is not None: @@ -78,7 +79,7 @@ class Process(object): return False time.sleep(0.1) - def terminate(self, timeout = 1.0): + def terminate(self, timeout=1.0): """Terminate a process, and all of its children that are in the same process group.""" try: @@ -89,19 +90,19 @@ class Process(object): def getpgid(pid): try: return os.getpgid(pid) - except OSError: # pragma: no cover + except OSError: # pragma: no cover return None def kill(pid, sig): try: return os.kill(pid, sig) - except OSError: # pragma: no cover + except OSError: # pragma: no cover return # Find all children group = getpgid(self._process.pid) main = psutil.Process(self._process.pid) - allproc = [ main ] + main.children(recursive = True) + allproc = [main] + main.children(recursive=True) # Kill with SIGTERM, if they're still in this process group for proc in allproc: @@ -119,7 +120,7 @@ class Process(object): # See if it worked return self._join(timeout) - except psutil.Error: # pragma: no cover (race condition) + except psutil.Error: # pragma: no cover (race condition) return True def clear_log(self): @@ -142,22 +143,22 @@ class Process(object): Call .get_info() about a second later.""" try: main = psutil.Process(self._process.pid) - self._process_list = [ main ] + main.children(recursive = True) + self._process_list = [main] + main.children(recursive=True) for proc in self._process_list: proc.cpu_percent(0) - except psutil.Error: # pragma: no cover (race condition) - self._process_list = [ ] + except psutil.Error: # pragma: no cover (race condition) + self._process_list = [] @staticmethod def get_empty_info(): - return { "cpu_percent": 0, - "cpu_user": 0, - "cpu_sys": 0, - "mem_phys": 0, - "mem_virt": 0, - "io_read": 0, - "io_write": 0, - "procs": 0 } + return {"cpu_percent": 0, + "cpu_user": 0, + "cpu_sys": 0, + "mem_phys": 0, + "mem_virt": 0, + "io_read": 0, + "io_write": 0, + "procs": 0} def get_info(self): """Return a dictionary with info about the process CPU and memory @@ -180,6 +181,7 @@ class Process(object): pass return d + class ProcessManager(object): """Track and manage a collection of Process objects""" def __init__(self): @@ -191,7 +193,7 @@ class ProcessManager(object): if pid in self.tmpdirs: try: shutil.rmtree(self.tmpdirs[pid]) - except OSError: # pragma: no cover + except OSError: # pragma: no cover pass del self.tmpdirs[pid] @@ -203,7 +205,7 @@ class ProcessManager(object): del self.processes[pid] shutil.rmtree(self.tmpdirs[pid]) del self.tmpdirs[pid] - except Exception: # pragma: no cover + except Exception: # pragma: no cover pass def __iter__(self): @@ -218,7 +220,7 @@ class ProcessManager(object): accessible in the code as sys.argv[1:].""" # The easiest way to do this, by far, is to just write the # code to a file. Make a directory to put it in. - tmpdir = tempfile.mkdtemp(prefix = "nilmrun-usercode-") + tmpdir = tempfile.mkdtemp(prefix="nilmrun-usercode-") try: # Write the code codepath = os.path.join(tmpdir, "usercode.py") @@ -229,7 +231,7 @@ class ProcessManager(object): f.write(repr(args)) # Run the code - argv = [ sys.executable, "-B", "-s", "-u", codepath ] + args + argv = [sys.executable, "-B", "-s", "-u", codepath] + args pid = self.run_command(argv) # Save the temp dir @@ -242,7 +244,7 @@ class ProcessManager(object): if tmpdir is not None: try: shutil.rmtree(tmpdir) - except OSError: # pragma: no cover + except OSError: # pragma: no cover pass def run_command(self, argv): @@ -260,15 +262,16 @@ class ProcessManager(object): def get_info(self): """Get info about all running PIDs""" - info = { "total" : Process.get_empty_info(), - "pids" : {}, - "system" : {} - } + info = { + "total": Process.get_empty_info(), + "pids": {}, + "system": {} + } # Trigger CPU usage collection for pid in self: self[pid].get_info_prepare() - psutil.cpu_percent(0, percpu = True) + psutil.cpu_percent(0, percpu=True) # Give it some time time.sleep(1) diff --git a/nilmrun/server.py b/nilmrun/server.py index 649648d..6eb31ce 100644 --- a/nilmrun/server.py +++ b/nilmrun/server.py @@ -1,18 +1,12 @@ """CherryPy-based server for running NILM filters via HTTP""" import cherrypy -import sys import os import socket -import json import traceback -import time -import nilmdb -from nilmdb.utils.printf import * +from nilmdb.utils.printf import sprintf from nilmdb.server.serverutil import ( - chunked_response, - response_type, exception_to_httperror, CORS_allow, json_to_request_params, @@ -23,10 +17,12 @@ from nilmdb.server.serverutil import ( ) from nilmdb.utils import serializer_proxy import nilmrun +import nilmrun.processmanager # Add CORS_allow tool cherrypy.tools.CORS_allow = cherrypy.Tool('on_start_resource', CORS_allow) + # CherryPy apps class App(object): """Root application for NILM runner""" @@ -53,6 +49,7 @@ class App(object): def version(self): return nilmrun.__version__ + class AppProcess(object): def __init__(self, manager): @@ -74,7 +71,7 @@ class AppProcess(object): # /process/status @cherrypy.expose @cherrypy.tools.json_out() - def status(self, pid, clear = False): + def status(self, pid, clear=False): """Return status about a process. If clear = True, also clear the log.""" clear = bool_param(clear) @@ -104,18 +101,19 @@ class AppProcess(object): @cherrypy.expose @cherrypy.tools.json_in() @cherrypy.tools.json_out() - @cherrypy.tools.CORS_allow(methods = ["POST"]) + @cherrypy.tools.CORS_allow(methods=["POST"]) def remove(self, pid): """Remove a process from the manager, killing it if necessary.""" if pid not in self.manager: raise cherrypy.HTTPError("404 Not Found", "No such PID") - if not self.manager.terminate(pid): # pragma: no cover + if not self.manager.terminate(pid): # pragma: no cover raise cherrypy.HTTPError("503 Service Unavailable", "Failed to stop process") status = self.process_status(pid) self.manager.remove(pid) return status + class AppRun(object): def __init__(self, manager): self.manager = manager @@ -125,7 +123,7 @@ class AppRun(object): @cherrypy.tools.json_in() @cherrypy.tools.json_out() @exception_to_httperror(nilmrun.processmanager.ProcessError) - @cherrypy.tools.CORS_allow(methods = ["POST"]) + @cherrypy.tools.CORS_allow(methods=["POST"]) def command(self, argv): """Execute an arbitrary program on the server. argv is a list of the program and its arguments: 'argv[0]' is the program @@ -140,8 +138,8 @@ class AppRun(object): @cherrypy.tools.json_in() @cherrypy.tools.json_out() @exception_to_httperror(nilmrun.processmanager.ProcessError) - @cherrypy.tools.CORS_allow(methods = ["POST"]) - def code(self, code, args = None): + @cherrypy.tools.CORS_allow(methods=["POST"]) + def code(self, code, args=None): """Execute arbitrary Python code. 'code' is a formatted string. It will be run as if it were written into a Python file and executed. 'args' is a list of strings, and they are passed @@ -154,10 +152,11 @@ class AppRun(object): "args must be a list of strings") return self.manager.run_code(code, args) + class Server(object): - def __init__(self, host = '127.0.0.1', port = 8080, - force_traceback = False, # include traceback in all errors - basepath = '', # base URL path for cherrypy.tree + def __init__(self, host='127.0.0.1', port=8080, + force_traceback=False, # include traceback in all errors + basepath='', # base URL path for cherrypy.tree ): # Build up global server configuration @@ -176,23 +175,25 @@ class Server(object): }) # Some default headers to just help identify that things are working - app_config.update({ 'response.headers.X-Jim-Is-Awesome': 'yeah' }) + app_config.update({'response.headers.X-Jim-Is-Awesome': 'yeah'}) # Set up Cross-Origin Resource Sharing (CORS) handler so we # can correctly respond to browsers' CORS preflight requests. # This also limits verbs to GET and HEAD by default. - app_config.update({ 'tools.CORS_allow.on': True, - 'tools.CORS_allow.methods': ['GET', 'HEAD'] }) + app_config.update({ + 'tools.CORS_allow.on': True, + 'tools.CORS_allow.methods': ['GET', 'HEAD'] + }) # Configure the 'json_in' tool to also allow other content-types # (like x-www-form-urlencoded), and to treat JSON as a dict that # fills requests.param. - app_config.update({ 'tools.json_in.force': False, - 'tools.json_in.processor': json_to_request_params }) + app_config.update({'tools.json_in.force': False, + 'tools.json_in.processor': json_to_request_params}) # Send tracebacks in error responses. They're hidden by the # error_page function for client errors (code 400-499). - app_config.update({ 'request.show_tracebacks' : True }) + app_config.update({'request.show_tracebacks': True}) self.force_traceback = force_traceback # Patch CherryPy error handler to never pad out error messages. @@ -210,7 +211,7 @@ class Server(object): root.process = AppProcess(manager) root.run = AppRun(manager) cherrypy.tree.apps = {} - cherrypy.tree.mount(root, basepath, config = { "/" : app_config }) + cherrypy.tree.mount(root, basepath, config={"/": app_config}) # Set up the WSGI application pointer for external programs self.wsgi_application = cherrypy.tree @@ -220,17 +221,20 @@ class Server(object): return json_error_page(status, message, traceback, version, self.force_traceback) - def start(self, blocking = False, event = None): + def start(self, blocking=False, event=None): cherrypy_start(blocking, event) def stop(self): cherrypy_stop() + # Multiple processes and threads should be OK here, but we'll still # follow the NilmDB approach of having just one globally initialized # copy of the server object. _wsgi_server = None -def wsgi_application(basepath): # pragma: no cover + + +def wsgi_application(basepath): # pragma: no cover """Return a WSGI application object. 'basepath' is the URL path of the application base, which @@ -243,12 +247,11 @@ def wsgi_application(basepath): # pragma: no cover # Try to start the server try: _wsgi_server = nilmrun.server.Server( - basepath = basepath.rstrip('/')) + basepath=basepath.rstrip('/')) except Exception: # Build an error message on failure import pprint - err = sprintf("Initializing nilmrun failed:\n\n", - dbpath) + err = "Initializing nilmrun failed:\n\n" err += traceback.format_exc() try: import pwd @@ -263,8 +266,10 @@ def wsgi_application(basepath): # pragma: no cover err += sprintf("\nEnvironment:\n%s\n", pprint.pformat(environ)) if _wsgi_server is None: # Serve up the error with our own mini WSGI app. - headers = [ ('Content-type', 'text/plain'), - ('Content-length', str(len(err))) ] + headers = [ + ('Content-type', 'text/plain'), + ('Content-length', str(len(err))) + ] start_response("500 Internal Server Error", headers) return [err] diff --git a/setup.cfg b/setup.cfg index f26c668..acde373 100644 --- a/setup.cfg +++ b/setup.cfg @@ -28,3 +28,11 @@ versionfile_source=nilmrun/_version.py versionfile_build=nilmrun/_version.py tag_prefix=nilmrun- parentdir_prefix=nilmrun- + +[flake8] +exclude=_version.py +extend-ignore=E731 + +[pylint] +ignore=_version.py +disable=C0103,C0111,R0913,R0914