From 518cf77b8c96d44c8c6208945e3c04e8c5e95605 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= <sybren@stuvel.eu> Date: Thu, 15 Nov 2018 10:43:39 +0100 Subject: [PATCH] Include mypy in development packages + solved lots of warnings Haven't written the mypy-running unit test yet, that'll come when all warnings are solved. This commit doesn't solve all of them, just some simple ones. --- .gitignore | 1 + Pipfile | 1 + Pipfile.lock | 51 ++++++++++++++++++++++-- flamenco_worker/commands.py | 30 +++++++------- flamenco_worker/config.py | 11 ++--- flamenco_worker/ssdp_discover.py | 11 ++++- flamenco_worker/tz.py | 4 +- flamenco_worker/upstream_update_queue.py | 2 + flamenco_worker/worker.py | 7 +--- 9 files changed, 87 insertions(+), 31 deletions(-) diff --git a/.gitignore b/.gitignore index af7e0a22..835ab56e 100644 --- a/.gitignore +++ b/.gitignore @@ -4,6 +4,7 @@ .cache/ .coverage +.mypy_cache/ /flamenco_worker.egg-info/ /flamenco-worker.db diff --git a/Pipfile b/Pipfile index 994ac947..531fd7ca 100644 --- a/Pipfile +++ b/Pipfile @@ -15,6 +15,7 @@ wheel = "*" pyinstaller = "*" ipython = "*" pathlib2 = {version = "*", markers="python_version < '3.6'"} +mypy = "*" [requires] python_version = "3.5" diff --git a/Pipfile.lock b/Pipfile.lock index 31e2c28c..cbb38b14 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -1,7 +1,7 @@ { "_meta": { "hash": { - "sha256": "ef7dc91825f95fb13543a1da77acb02ef4066d89922eed4d7e962b0a57032307" + "sha256": "e426596954522d58f3248f5f4651067486da5aff3799d5568f8843289fdc4de8" }, "pipfile-spec": 6, "requires": { @@ -192,6 +192,21 @@ ], "version": "==4.3.0" }, + "mypy": { + "hashes": [ + "sha256:8e071ec32cc226e948a34bbb3d196eb0fd96f3ac69b6843a5aff9bd4efa14455", + "sha256:fb90c804b84cfd8133d3ddfbd630252694d11ccc1eb0166a1b2efb5da37ecab2" + ], + "index": "pypi", + "version": "==0.641" + }, + "mypy-extensions": { + "hashes": [ + "sha256:37e0e956f41369209a3d5f34580150bcacfabaa57b33a15c0b25f4b5725e0812", + "sha256:b16cabe759f55e3409a7d231ebd2841378fb0c27a5d1994719e340e4f429ac3e" + ], + "version": "==0.4.1" + }, "parso": { "hashes": [ "sha256:35704a43a3c113cce4de228ddb39aab374b8004f4f2407d070b6a2ca784ce8a2", @@ -274,11 +289,11 @@ }, "pytest": { "hashes": [ - "sha256:3f193df1cfe1d1609d4c583838bea3d532b18d6160fd3f55c9447fdca30848ec", - "sha256:e246cf173c01169b9617fc07264b7b1316e78d7a650055235d6d897bc80d9660" + "sha256:488c842647bbeb350029da10325cb40af0a9c7a2fdda45aeb1dda75b60048ffb", + "sha256:c055690dfefa744992f563e8c3a654089a6aa5b8092dded9b6fafbd70b2e45a7" ], "index": "pypi", - "version": "==3.10.1" + "version": "==4.0.0" }, "pytest-cov": { "hashes": [ @@ -302,6 +317,34 @@ ], "version": "==4.3.2" }, + "typed-ast": { + "hashes": [ + "sha256:0948004fa228ae071054f5208840a1e88747a357ec1101c17217bfe99b299d58", + "sha256:10703d3cec8dcd9eef5a630a04056bbc898abc19bac5691612acba7d1325b66d", + "sha256:1f6c4bd0bdc0f14246fd41262df7dfc018d65bb05f6e16390b7ea26ca454a291", + "sha256:25d8feefe27eb0303b73545416b13d108c6067b846b543738a25ff304824ed9a", + "sha256:29464a177d56e4e055b5f7b629935af7f49c196be47528cc94e0a7bf83fbc2b9", + "sha256:2e214b72168ea0275efd6c884b114ab42e316de3ffa125b267e732ed2abda892", + "sha256:3e0d5e48e3a23e9a4d1a9f698e32a542a4a288c871d33ed8df1b092a40f3a0f9", + "sha256:519425deca5c2b2bdac49f77b2c5625781abbaf9a809d727d3a5596b30bb4ded", + "sha256:57fe287f0cdd9ceaf69e7b71a2e94a24b5d268b35df251a88fef5cc241bf73aa", + "sha256:668d0cec391d9aed1c6a388b0d5b97cd22e6073eaa5fbaa6d2946603b4871efe", + "sha256:68ba70684990f59497680ff90d18e756a47bf4863c604098f10de9716b2c0bdd", + "sha256:6de012d2b166fe7a4cdf505eee3aaa12192f7ba365beeefaca4ec10e31241a85", + "sha256:79b91ebe5a28d349b6d0d323023350133e927b4de5b651a8aa2db69c761420c6", + "sha256:8550177fa5d4c1f09b5e5f524411c44633c80ec69b24e0e98906dd761941ca46", + "sha256:898f818399cafcdb93cbbe15fc83a33d05f18e29fb498ddc09b0214cdfc7cd51", + "sha256:94b091dc0f19291adcb279a108f5d38de2430411068b219f41b343c03b28fb1f", + "sha256:a26863198902cda15ab4503991e8cf1ca874219e0118cbf07c126bce7c4db129", + "sha256:a8034021801bc0440f2e027c354b4eafd95891b573e12ff0418dec385c76785c", + "sha256:bc978ac17468fe868ee589c795d06777f75496b1ed576d308002c8a5756fb9ea", + "sha256:c05b41bc1deade9f90ddc5d988fe506208019ebba9f2578c622516fd201f5863", + "sha256:c9b060bd1e5a26ab6e8267fd46fc9e02b54eb15fffb16d112d4c7b1c12987559", + "sha256:edb04bdd45bfd76c8292c4d9654568efaedf76fe78eb246dde69bdb13b2dad87", + "sha256:f19f2a4f547505fe9072e15f6f4ae714af51b5a681a97f187971f50c283193b6" + ], + "version": "==1.1.0" + }, "wcwidth": { "hashes": [ "sha256:3df37372226d6e63e1b1e1eda15c594bca98a22d33a23832a90998faa96bc65e", diff --git a/flamenco_worker/commands.py b/flamenco_worker/commands.py index 33f7f6dc..52309aa5 100644 --- a/flamenco_worker/commands.py +++ b/flamenco_worker/commands.py @@ -20,7 +20,7 @@ import psutil from . import worker -command_handlers = {} +command_handlers = {} # type: typing.Mapping[str, typing.Type['AbstractCommand']] # Timeout of subprocess.stdout.readline() call. SUBPROC_READLINE_TIMEOUT = 3600 # seconds @@ -82,8 +82,8 @@ class AbstractCommand(metaclass=abc.ABCMeta): # Set by __attr_post_init__() identifier = attr.ib(default=None, init=False, validator=attr.validators.optional(attr.validators.instance_of(str))) - _log = attr.ib(default=None, init=False, - validator=attr.validators.optional(attr.validators.instance_of(logging.Logger))) + _log = attr.ib(init=False, default=logging.getLogger('AbstractCommand'), + validator=attr.validators.instance_of(logging.Logger)) def __attrs_post_init__(self): self.identifier = '%s.(task_id=%s, command_idx=%s)' % ( @@ -174,8 +174,8 @@ class AbstractCommand(metaclass=abc.ABCMeta): return None - def _setting(self, settings: dict, key: str, is_required: bool, valtype: typing.Type = str) -> ( - typing.Any, typing.Optional[str]): + def _setting(self, settings: dict, key: str, is_required: bool, valtype: typing.Type = str) \ + -> typing.Tuple[typing.Any, typing.Optional[str]]: """Parses a setting, returns either (value, None) or (None, errormsg)""" try: @@ -274,10 +274,10 @@ def _unique_path(path: Path) -> Path: suffix = m.group(1) try: - suffix = int(suffix) + suffix_value = int(suffix) except ValueError: continue - max_nr = max(max_nr, suffix) + max_nr = max(max_nr, suffix_value) return path.with_name(path.name + '~%i' % (max_nr + 1)) @@ -472,20 +472,22 @@ class AbstractSubprocessCommand(AbstractCommand): pidfile.write(str(pid)) try: + assert self.proc.stdout is not None + while not self.proc.stdout.at_eof(): try: - line = await asyncio.wait_for(self.proc.stdout.readline(), - self.readline_timeout) + line_bytes = await asyncio.wait_for(self.proc.stdout.readline(), + self.readline_timeout) except asyncio.TimeoutError: raise CommandExecutionError('Command pid=%d timed out after %i seconds' % (pid, self.readline_timeout)) - if len(line) == 0: + if len(line_bytes) == 0: # EOF received, so let's bail. break try: - line = line.decode('utf8') + line = line_bytes.decode('utf8') except UnicodeDecodeError as ex: await self.abort() raise CommandExecutionError( @@ -493,9 +495,9 @@ class AbstractSubprocessCommand(AbstractCommand): line = line.rstrip() self._log.debug('Read line pid=%d: %s', pid, line) - line = await self.process_line(line) - if line is not None: - await self.worker.register_log(line) + processed_line = await self.process_line(line) + if processed_line is not None: + await self.worker.register_log(processed_line) retcode = await self.proc.wait() self._log.info('Command %r (pid=%d) stopped with status code %s', args, pid, retcode) diff --git a/flamenco_worker/config.py b/flamenco_worker/config.py index a8523c30..6b75d9fa 100644 --- a/flamenco_worker/config.py +++ b/flamenco_worker/config.py @@ -5,6 +5,7 @@ import configparser import datetime import pathlib import logging +import typing from . import worker @@ -28,7 +29,7 @@ DEFAULT_CONFIG = { ('push_act_max_interval_seconds', str(worker.PUSH_ACT_MAX_INTERVAL.total_seconds())), ]), 'pre_task_check': collections.OrderedDict([]), -} +} # type: typing.Mapping[str, typing.Mapping[str, typing.Any]] # Will be assigned to the config key 'task_types' when started with --test CLI arg. TESTING_TASK_TYPES = 'test-blender-render' @@ -53,8 +54,8 @@ class ConfigParser(configparser.ConfigParser): secs = self.value(key, float) return datetime.timedelta(seconds=secs) - def erase(self, key: str) -> bool: - return self.set(CONFIG_SECTION, key, '') + def erase(self, key: str) -> None: + self.set(CONFIG_SECTION, key, '') def merge_with_home_config(new_conf: dict): @@ -93,12 +94,12 @@ def load_config(config_file: pathlib.Path = None, if config_file: log.info('Loading configuration from %s', config_file) if not config_file.exists(): - log.fatal('Config file %s does not exist', config_file) + log.error('Config file %s does not exist', config_file) raise SystemExit(47) loaded = confparser.read(str(config_file), encoding='utf8') else: if not GLOBAL_CONFIG_FILE.exists(): - log.fatal('Config file %s does not exist', GLOBAL_CONFIG_FILE) + log.error('Config file %s does not exist', GLOBAL_CONFIG_FILE) raise SystemExit(47) config_files = [GLOBAL_CONFIG_FILE, HOME_CONFIG_FILE] diff --git a/flamenco_worker/ssdp_discover.py b/flamenco_worker/ssdp_discover.py index c1f6c18d..29e6c9a8 100644 --- a/flamenco_worker/ssdp_discover.py +++ b/flamenco_worker/ssdp_discover.py @@ -30,9 +30,16 @@ class Response(HTTPResponse): self.fp = BytesIO(payload) self.debuglevel = 0 self.strict = 0 - self.headers = self.msg = None + + # This is also done in the HTTPResponse __init__ function, but + # MyPy still doesn't like it. + self.headers = self.msg = None # type: ignore + self._method = None - self.begin() + + # This function is available on the superclass, but still + # MyPy doesn't think it is. + self.begin() # type: ignore def interface_addresses(): diff --git a/flamenco_worker/tz.py b/flamenco_worker/tz.py index 25ea8c9b..4bc360d8 100644 --- a/flamenco_worker/tz.py +++ b/flamenco_worker/tz.py @@ -22,7 +22,9 @@ class tzutc(datetime.tzinfo): return True - __hash__ = None + # Assigning a different type than object.__hash__ (None resp. Callable) + # is not allowed by MyPy. Here it's intentional, though. + __hash__ = None # type: ignore def __ne__(self, other): return not (self == other) diff --git a/flamenco_worker/upstream_update_queue.py b/flamenco_worker/upstream_update_queue.py index 18999f55..e33e3ea5 100644 --- a/flamenco_worker/upstream_update_queue.py +++ b/flamenco_worker/upstream_update_queue.py @@ -98,6 +98,7 @@ class TaskUpdateQueue: if self._db is None: self._connect_db() + assert self._db is not None result = self._db.execute(''' SELECT rowid, url, payload @@ -114,6 +115,7 @@ class TaskUpdateQueue: """Return the number of items queued.""" if self._db is None: self._connect_db() + assert self._db is not None result = self._db.execute('SELECT count(*) FROM fworker_queue') count = next(result)[0] diff --git a/flamenco_worker/worker.py b/flamenco_worker/worker.py index 02fc46e7..4f10e5bc 100644 --- a/flamenco_worker/worker.py +++ b/flamenco_worker/worker.py @@ -4,9 +4,11 @@ import enum import itertools import pathlib import tempfile +import traceback import typing import attr +import requests.exceptions from . import attrs_extra from . import documents @@ -184,8 +186,6 @@ class FlamencoWorker: async def _keep_posting_to_manager(self, url: str, json: dict, *, use_auth=True, may_retry_loop: bool): - import requests - post_kwargs = { 'json': json, 'loop': self.loop, @@ -370,9 +370,6 @@ class FlamencoWorker: :param delay: waits this many seconds before fetching a task. """ - import traceback - import requests - self.state = WorkerState.AWAKE self._cleanup_state_for_new_task() -- GitLab