From 4c14e35a7807d5be63e7a6f0f5d382afcc8c1f20 Mon Sep 17 00:00:00 2001 From: Joel Collins Date: Tue, 24 Nov 2020 17:46:13 +0000 Subject: [PATCH] Added PyLint analysis --- .deepsource.toml | 19 ------- .github/workflows/codeql-analysis.yml | 51 ------------------- .github/workflows/publish.yml | 3 ++ .github/workflows/test.yml | 3 ++ pyproject.toml | 9 +++- src/labthings/actions/thread.py | 9 ++-- src/labthings/apispec/plugins.py | 4 +- src/labthings/default_views/events.py | 1 - src/labthings/extensions.py | 26 +++++++--- src/labthings/find.py | 2 +- .../json/marshmallow_jsonschema/base.py | 7 +-- .../json/marshmallow_jsonschema/exceptions.py | 4 +- src/labthings/json/schemas.py | 5 +- src/labthings/labthing.py | 13 ++--- src/labthings/marshalling/args.py | 2 +- src/labthings/marshalling/marshalling.py | 4 +- src/labthings/monkey.py | 2 +- src/labthings/schema.py | 10 ++-- src/labthings/sync/event.py | 2 +- src/labthings/sync/lock.py | 8 ++- src/labthings/utilities.py | 4 +- src/labthings/views/__init__.py | 10 +--- src/labthings/views/builder.py | 2 +- src/labthings/wsgi.py | 6 +-- tests/test_tasks_thread.py | 2 +- 25 files changed, 77 insertions(+), 131 deletions(-) delete mode 100644 .deepsource.toml delete mode 100644 .github/workflows/codeql-analysis.yml diff --git a/.deepsource.toml b/.deepsource.toml deleted file mode 100644 index bbae29e9..00000000 --- a/.deepsource.toml +++ /dev/null @@ -1,19 +0,0 @@ -version = 1 - -# Glob pattern of the test files -test_patterns = [ - "tests/**", - "test_*.py" -] - -[[analyzers]] -name = "python" -enabled = true - -dependency_file_paths = [ - "poetry.lock", - "pyproject.toml" -] - - [analyzers.meta] - runtime_version = "3.x.x" diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml deleted file mode 100644 index 22a75f65..00000000 --- a/.github/workflows/codeql-analysis.yml +++ /dev/null @@ -1,51 +0,0 @@ -name: "Code scanning - action" - -on: - push: - pull_request: - schedule: - - cron: '0 4 * * 3' - -jobs: - CodeQL-Build: - - runs-on: ubuntu-latest - - steps: - - name: Checkout repository - uses: actions/checkout@v2 - with: - # We must fetch at least the immediate parents so that if this is - # a pull request then we can checkout the head. - fetch-depth: 2 - - # If this run was triggered by a pull request event, then checkout - # the head of the pull request instead of the merge commit. - - run: git checkout HEAD^2 - if: ${{ github.event_name == 'pull_request' }} - - # Initializes the CodeQL tools for scanning. - - name: Initialize CodeQL - uses: github/codeql-action/init@v1 - # Override language selection by uncommenting this and choosing your languages - # with: - # languages: go, javascript, csharp, python, cpp, java - - # Autobuild attempts to build any compiled languages (C/C++, C#, or Java). - # If this step fails, then you should remove it and run the build manually (see below) - - name: Autobuild - uses: github/codeql-action/autobuild@v1 - - # ℹī¸ Command-line programs to run using the OS shell. - # 📚 https://git.io/JvXDl - - # ✏ī¸ If the Autobuild fails above, remove it and uncomment the following three lines - # and modify them (or add more) to build your code if your project - # uses a compiled language - - #- run: | - # make bootstrap - # make release - - - name: Perform CodeQL Analysis - uses: github/codeql-action/analyze@v1 diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 1d8d6069..8fd7c2b2 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -37,6 +37,9 @@ jobs: - name: Analyse with MyPy run: poetry run mypy src + - name: Lint with PyLint + run: poetry run pylint .\src\labthings\ + - name: Test with pytest run: poetry run pytest diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index dd604243..bcb5634f 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -39,6 +39,9 @@ jobs: - name: Analyse with MyPy run: poetry run mypy src + - name: Lint with PyLint + run: poetry run pylint .\src\labthings\ + - name: Test with pytest run: poetry run pytest diff --git a/pyproject.toml b/pyproject.toml index be81a692..fd4d7828 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -34,7 +34,7 @@ sphinx-rtd-theme = "^0.5.0" mypy = "^0.790" [tool.black] -exclude = '(\.eggs|\.git|\.venv)' +exclude = '(\.eggs|\.git|\.venv|node_modules/)' [tool.isort] multi_line_output = 3 @@ -44,6 +44,13 @@ use_parentheses = true ensure_newline_before_comments = true line_length = 88 +[tool.pylint.'MESSAGES CONTROL'] +disable = "fixme,C,R" +max-line-length = 88 + +[tool.pylint.'MASTER'] +ignore = "marshmallow_jsonschema" + [build-system] requires = ["poetry>=0.12"] build-backend = "poetry.masonry.api" diff --git a/src/labthings/actions/thread.py b/src/labthings/actions/thread.py index 6c961c3c..b984a081 100644 --- a/src/labthings/actions/thread.py +++ b/src/labthings/actions/thread.py @@ -70,7 +70,7 @@ def __init__( # copy_current_request_context allows threads to access flask current_app if has_request_context(): - logging.debug(f"Copying request context to {self._target}") + logging.debug("Copying request context to %s", self._target) self._target = copy_current_request_context(self._target) try: self.input = request.json @@ -298,7 +298,7 @@ def terminate(self, exception=ActionKilledException) -> bool: :raises which: should cause the thread to exit silently """ - _LOG.warning(f"Terminating thread {self}") + _LOG.warning("Terminating thread %s", self) if not (self.is_alive() or self._is_thread_proc_running()): logging.debug("Cannot kill thread that is no longer running.") return False @@ -336,7 +336,7 @@ def stop(self, timeout=None, exception=ActionKilledException) -> bool: self._status = "cancelled" return True # If the timeout tracker stopped before the thread died, kill it - logging.warning(f"Forcefully terminating thread {self}") + logging.warning("Forcefully terminating thread %s", self) return self.terminate(exception=exception) @@ -346,7 +346,6 @@ def __init__( thread: ActionThread, dest: LockableDeque, level=logging.INFO, - default_log_len: int = 100, ): """Set up a log handler that appends messages to a list. @@ -371,7 +370,7 @@ def __init__( self.dest = dest self.addFilter(self.check_thread) - def check_thread(self, record): + def check_thread(self, *_): """Determine if a thread matches the desired record :param record: diff --git a/src/labthings/apispec/plugins.py b/src/labthings/apispec/plugins.py index e83afda7..ce335535 100644 --- a/src/labthings/apispec/plugins.py +++ b/src/labthings/apispec/plugins.py @@ -25,13 +25,14 @@ def init_attribute_functions(self, *args, **kwargs): OpenAPIConverter.init_attribute_functions(self, *args, **kwargs) self.attribute_functions.append(self.jsonschema_type_mapping) - def jsonschema_type_mapping(self, field, **kwargs): + def jsonschema_type_mapping(self, field, **_): """ :param field: :param **kwargs: """ ret = {} if hasattr(field, "_jsonschema_type_mapping"): + # pylint: disable=protected-access schema = field._jsonschema_type_mapping() ret.update(schema) return ret @@ -246,6 +247,7 @@ def spec_for_event(cls, event): ) return d + # pylint: disable=signature-differs def operation_helper(self, path, operations, **kwargs): """Path helper that allows passing a Flask view function.""" # rule = self._rule_for_view(interaction.dispatch_request, app=app) diff --git a/src/labthings/default_views/events.py b/src/labthings/default_views/events.py index fa39c890..65c38783 100644 --- a/src/labthings/default_views/events.py +++ b/src/labthings/default_views/events.py @@ -1,4 +1,3 @@ -from .. import fields from ..schema import LogRecordSchema from ..views import EventView diff --git a/src/labthings/extensions.py b/src/labthings/extensions.py index b8998038..517dcb2c 100644 --- a/src/labthings/extensions.py +++ b/src/labthings/extensions.py @@ -7,7 +7,7 @@ from flask import url_for -from typing import List, Dict, Callable +from typing import List, Dict, Callable, Union from .utilities import camel_to_snake, get_docstring, snake_to_spine from .views.builder import static_from @@ -36,11 +36,11 @@ def __init__( self._meta: dict = {} # Extra metadata to add to the extension description self._on_registers: List[ - Dict + Union[Dict, Callable] ] = [] # List of dictionaries of functions to run on registration self._on_components: List[ - Dict + Union[Dict, Callable] ] = [] # List of dictionaries of functions to run as components are added self._cls = str(self) # String description of extension instance @@ -64,6 +64,14 @@ def views(self): """ """ return self._views + @property + def on_components(self): + return self._on_components + + @property + def on_registers(self): + return self._on_registers + def add_view(self, view_class, *urls, endpoint=None, **kwargs): """ @@ -217,7 +225,7 @@ def find_instances_in_module(module, class_to_find): for attribute in dir(module): if not attribute.startswith("__"): if isinstance(getattr(module, attribute), class_to_find): - logging.debug(f"Found extension {getattr(module, attribute).name}") + logging.debug("Found extension %s", getattr(module, attribute).name) objs.append(getattr(module, attribute)) return objs @@ -235,7 +243,7 @@ def find_extensions_in_file(extension_path: str, module_name="extensions") -> li :rtype: list """ - logging.debug(f"Loading extensions from {extension_path}") + logging.debug("Loading extensions from %s", extension_path) spec = util.spec_from_file_location(module_name, extension_path) mod = util.module_from_spec(spec) @@ -243,9 +251,11 @@ def find_extensions_in_file(extension_path: str, module_name="extensions") -> li try: spec.loader.exec_module(mod) # type: ignore - except Exception: # skipcq: PYL-W0703 + except Exception: # pylint: disable=broad-except logging.error( - f"Exception in extension path {extension_path}: \n{traceback.format_exc()}" + "Exception in extension path %s: \n%s", + extension_path, + traceback.format_exc(), ) return [] else: @@ -270,7 +280,7 @@ def find_extensions(extension_dir: str, module_name="extensions") -> list: :rtype: list """ - logging.debug(f"Loading extensions from {extension_dir}") + logging.debug("Loading extensions from %s", extension_dir) extensions = [] extension_paths = glob.glob(os.path.join(extension_dir, "*.py")) diff --git a/src/labthings/find.py b/src/labthings/find.py index a9af62fe..1f31fbe8 100644 --- a/src/labthings/find.py +++ b/src/labthings/find.py @@ -27,7 +27,7 @@ def current_labthing(app=None): # reach the Flask app object. Just using current_app returns # a wrapper, which breaks it's use in Task threads try: - app = current_app._get_current_object() # skipcq: PYL-W0212 + app = current_app._get_current_object() # pylint: disable=protected-access except RuntimeError: return None ext = app.extensions.get(EXTENSION_NAME, None) diff --git a/src/labthings/json/marshmallow_jsonschema/base.py b/src/labthings/json/marshmallow_jsonschema/base.py index 54d31188..370a9941 100644 --- a/src/labthings/json/marshmallow_jsonschema/base.py +++ b/src/labthings/json/marshmallow_jsonschema/base.py @@ -204,7 +204,7 @@ def _get_schema_for_field(self, obj, field): schema = FIELD_VALIDATORS[base_class](schema, field, validator, obj) return schema - def _from_nested_schema(self, obj, field): + def _from_nested_schema(self, _, field): """Support nested field. :param obj: @@ -217,17 +217,12 @@ def _from_nested_schema(self, obj, field): nested = field.nested if isclass(nested) and issubclass(nested, Schema): - name = nested.__name__ only = field.only exclude = field.exclude - nested_cls = nested nested_instance = nested(only=only, exclude=exclude) else: - nested_cls = nested.__class__ - name = nested_cls.__name__ nested_instance = nested - outer_name = obj.__class__.__name__ # If this is not a schema we've seen, and it's not this schema (checking this for recursive schemas), # put it in our list of schema defs wrapped_nested = self.__class__(nested=True) diff --git a/src/labthings/json/marshmallow_jsonschema/exceptions.py b/src/labthings/json/marshmallow_jsonschema/exceptions.py index 67bf0d84..7e8410f9 100644 --- a/src/labthings/json/marshmallow_jsonschema/exceptions.py +++ b/src/labthings/json/marshmallow_jsonschema/exceptions.py @@ -1,4 +1,2 @@ class UnsupportedValueError(Exception): - """ """ - - pass + """ """ \ No newline at end of file diff --git a/src/labthings/json/schemas.py b/src/labthings/json/schemas.py index 332f1c46..ea248c54 100644 --- a/src/labthings/json/schemas.py +++ b/src/labthings/json/schemas.py @@ -68,7 +68,7 @@ def argument_to_param( """ param: Dict[str, Any] = {"in": "path", "name": argument, "required": True} type_, format_ = CONVERTER_MAPPING.get( - # skipcq: PYL-W0212 + # pylint: disable=protected-access type(rule._converters[argument]), # type: ignore DEFAULT_TYPE, ) @@ -88,6 +88,7 @@ def field_to_property(field: fields.Field): :param field: """ + # pylint: disable=protected-access return JSONSchema()._get_schema_for_field(Schema(), field) @@ -103,7 +104,7 @@ def schema_to_json( return None if isinstance(schema, fields.Field): return field_to_property(schema) - elif isinstance(schema, Dict): + elif isinstance(schema, dict): return JSONSchema().dump(Schema.from_dict(schema)()) elif isinstance(schema, Schema): return JSONSchema().dump(schema) diff --git a/src/labthings/labthing.py b/src/labthings/labthing.py index 4f6711a3..1eea75c4 100644 --- a/src/labthings/labthing.py +++ b/src/labthings/labthing.py @@ -287,7 +287,7 @@ def dummy(*_): for extension_object in self.extensions.values(): # For each on_component function - for com_func in extension_object._on_components: + for com_func in extension_object.on_components: # If the component matches if com_func.get("component", "") == component_name: # Call the function @@ -332,14 +332,14 @@ def dummy(*_): ) # For each on_register function - for reg_func in extension_object._on_registers: + for reg_func in extension_object.on_registers: # Call the function reg_func.get("function", dummy)( *reg_func.get("args"), **reg_func.get("kwargs") ) # For each on_component function - for com_func in extension_object._on_components: + for com_func in extension_object.on_components: key = com_func.get("component", "") # If the component has already been added if key in self.components: @@ -393,7 +393,7 @@ def add_view( """ endpoint = endpoint or snake_to_camel(view.__name__) - logging.debug(f"{endpoint}: {type(view)} @ {urls}") + logging.debug("%s: %s @ %s", endpoint, type(view), urls) if self.app is not None: self._register_view(self.app, view, *urls, endpoint=endpoint, **kwargs) @@ -449,8 +449,9 @@ def _register_view( app.add_url_rule(rule, view_func=resource_func, endpoint=endpoint, **kwargs) # There might be a better way to do this than _rules_by_endpoint, - # but I can't find one so this will do for now. Skipping PYL-W0212 - flask_rules = app.url_map._rules_by_endpoint.get(endpoint) # skipcq: PYL-W0212 + # but I can't find one so this will do for now. + # pylint: disable=protected-access + flask_rules = app.url_map._rules_by_endpoint.get(endpoint) with app.test_request_context(): self.spec.path(view=resource_func, interaction=view) diff --git a/src/labthings/marshalling/args.py b/src/labthings/marshalling/args.py index 044128f3..fccf8a31 100644 --- a/src/labthings/marshalling/args.py +++ b/src/labthings/marshalling/args.py @@ -15,7 +15,7 @@ class use_body: """Gets the request body as a single value and adds it as a positional argument""" def __init__( - self, schema: Union[Schema, Field, Dict[str, Union[Field, type]]], **kwargs + self, schema: Union[Schema, Field, Dict[str, Union[Field, type]]], **_ ): self.schema = schema diff --git a/src/labthings/marshalling/marshalling.py b/src/labthings/marshalling/marshalling.py index b6ea303c..b6dffdb4 100644 --- a/src/labthings/marshalling/marshalling.py +++ b/src/labthings/marshalling/marshalling.py @@ -1,6 +1,5 @@ from collections.abc import Mapping from functools import wraps -from flask.wrappers import Response from marshmallow import Schema as _Schema from werkzeug.wrappers import Response as ResponseBase @@ -21,6 +20,9 @@ def schema_to_converter(schema: Union[Schema, Field, Dict[str, Union[Field, type """ if isinstance(schema, Mapping): + # Please ignore the pylint disable below, + # GeneratedSchema definitely does have a `dump` member + # pylint: disable=no-member return Schema.from_dict(schema)().dump # Case of schema as a single Field elif isinstance(schema, Field): diff --git a/src/labthings/monkey.py b/src/labthings/monkey.py index 569890ff..ad036dec 100644 --- a/src/labthings/monkey.py +++ b/src/labthings/monkey.py @@ -1,7 +1,7 @@ import logging -def patch_all(*args, **kwargs): +def patch_all(*_): """ :param *args: diff --git a/src/labthings/schema.py b/src/labthings/schema.py index c0249d43..31d2da0a 100644 --- a/src/labthings/schema.py +++ b/src/labthings/schema.py @@ -1,6 +1,6 @@ # -*- coding: utf-8 -*- import logging -from typing import Optional, Union, Dict +from typing import Optional, Union, Dict, Any from datetime import datetime from flask import url_for @@ -56,7 +56,7 @@ def serialize(self, value): return self.field.serialize("value", obj) - def dump(self, value): + def dump(self, value: Any, *_): # pylint: disable=arguments-differ """ :param value: @@ -75,7 +75,7 @@ class LogRecordSchema(Schema): created = fields.DateTime() @pre_dump - def preprocess(self, data, **kwargs): + def preprocess(self, data, **_): if isinstance(data, logging.LogRecord): data.message = data.getMessage() if not isinstance(data.created, datetime): @@ -105,7 +105,7 @@ class ActionSchema(Schema): links = fields.Dict() @pre_dump - def generate_links(self, data, **kwargs): + def generate_links(self, data, **_): """ :param data: @@ -195,7 +195,7 @@ class ExtensionSchema(Schema): links = fields.Dict() @pre_dump - def generate_links(self, data, **kwargs): + def generate_links(self, data, **_): """ :param data: diff --git a/src/labthings/sync/event.py b/src/labthings/sync/event.py index c8a54a9c..fb6e02ba 100644 --- a/src/labthings/sync/event.py +++ b/src/labthings/sync/event.py @@ -62,7 +62,7 @@ def clear(self) -> bool: """Clear frame event, once processed.""" ident = get_ident() if ident not in self.events: - logging.error(f"Mismatched ident. Current: {ident}, available:") + logging.error("Mismatched ident. Current: %s, available:", ident) logging.error(self.events.keys()) return False self.events[get_ident()][0].clear() diff --git a/src/labthings/sync/lock.py b/src/labthings/sync/lock.py index 3f94ac5b..5aff0179 100644 --- a/src/labthings/sync/lock.py +++ b/src/labthings/sync/lock.py @@ -48,6 +48,7 @@ def __init__(self, timeout: int = -1, name: Optional[str] = None): @property def _owner(self): """ """ + # pylint: disable=protected-access return self._lock._owner @contextmanager @@ -59,6 +60,7 @@ def __call__(self, timeout=sentinel, blocking: bool = True): def locked(self): """ """ + # pylint: disable=protected-access return bool(self._lock._count) def acquire(self, blocking: bool = True, timeout=sentinel, _strict: bool = True): @@ -93,6 +95,7 @@ def release(self): def _is_owned(self): """ """ + # pylint: disable=protected-access return self._lock._is_owned() @@ -114,6 +117,7 @@ def __init__(self, locks, timeout: int = -1): @property def _owner(self): """ """ + # pylint: disable=protected-access return [lock._owner for lock in self.locks] @contextmanager @@ -144,7 +148,7 @@ def acquire(self, blocking: bool = True, timeout=sentinel): if not lock_all: self._emergency_release() - logging.error(f"Unable to acquire {self} within {timeout} seconds") + logging.error("Unable to acquire %s within %s seconds", self, timeout) raise LockError("ACQUIRE_ERROR", self) return True @@ -167,6 +171,7 @@ def release(self): def _emergency_release(self): """ """ for lock in self.locks: + # pylint: disable=protected-access if lock.locked() and lock._is_owned(): lock.release() @@ -176,4 +181,5 @@ def locked(self): def _is_owned(self): """ """ + # pylint: disable=protected-access return all(lock._is_owned() for lock in self.locks) diff --git a/src/labthings/utilities.py b/src/labthings/utilities.py index fe080ff1..904f8e8a 100644 --- a/src/labthings/utilities.py +++ b/src/labthings/utilities.py @@ -7,7 +7,7 @@ from collections import UserString from functools import reduce -from typing import Type, Callable, Dict, Any, Tuple, Union, List, Iterable +from typing import Type, Callable, Dict, Any, Tuple, Union, List from flask import current_app, has_request_context, request from werkzeug.http import HTTP_STATUS_CODES @@ -228,7 +228,7 @@ def rapply( """ # If the object is a dictionary - if isinstance(data, Dict): + if isinstance(data, dict): return { key: rapply( val, func, *args, apply_to_iterables=apply_to_iterables, **kwargs diff --git a/src/labthings/views/__init__.py b/src/labthings/views/__init__.py index 6bc6d5d2..a7cbfffa 100644 --- a/src/labthings/views/__init__.py +++ b/src/labthings/views/__init__.py @@ -1,10 +1,8 @@ import datetime -import threading from collections import OrderedDict -from flask import abort, request +from flask import request from flask.views import MethodView -from werkzeug.exceptions import BadRequest from werkzeug.wrappers import Response as ResponseBase from typing import Dict, Optional, Set, List @@ -142,9 +140,6 @@ class ActionView(View): Pool() ) # Emergency thread pool (common to all ActionView subclasses) - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) - def __init_subclass__(cls): """ Here we handle all class attributes that should be specific to each subclass of ActionView. @@ -264,9 +259,6 @@ class EventView(View): _cls_tags = {"events"} _deque = Deque() # Action queue - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) - @classmethod def get(cls): """ diff --git a/src/labthings/views/builder.py b/src/labthings/views/builder.py index c2fa6cf4..4027747c 100644 --- a/src/labthings/views/builder.py +++ b/src/labthings/views/builder.py @@ -20,7 +20,7 @@ def static_from(static_folder: str, name=None) -> Type[MethodView]: name = f"static-{uid}" # Create inner functions - def _get(self, path=""): + def _get(_, path=""): """ :param path: (Default value = "") """ diff --git a/src/labthings/wsgi.py b/src/labthings/wsgi.py index 693e72d4..10199a4c 100644 --- a/src/labthings/wsgi.py +++ b/src/labthings/wsgi.py @@ -22,9 +22,7 @@ class Server: :type zeroconf: bool """ - def __init__( - self, app, host="0.0.0.0", port=7485, debug=False, zeroconf=True, **kwargs - ): + def __init__(self, app, host="0.0.0.0", port=7485, debug=False, zeroconf=True): self.app = app # Find LabThing attached to app with app.app_context(): @@ -103,7 +101,7 @@ def start(self): self.zeroconf_server.close() print("Server stopped") - def run(self, host=None, port=None, debug=None, zeroconf=None, **kwargs): + def run(self, host=None, port=None, debug=None, zeroconf=None): """Starts the server allowing for runtime parameters. Designed to immitate the old Flask app.run style of starting an app diff --git a/tests/test_tasks_thread.py b/tests/test_tasks_thread.py index eef195b8..f644d1b2 100644 --- a/tests/test_tasks_thread.py +++ b/tests/test_tasks_thread.py @@ -191,4 +191,4 @@ def test_task_log_with_incorrect_thread(): # Should always return False if called from outside the log handlers thread assert task_log_handler.thread == task_obj - assert not task_log_handler.check_thread(record=None) + assert not task_log_handler.check_thread()