Introduce mypy overrides

.pre-commit-config.yaml

@@ -22,7 +22,13 @@ repos:
     rev: v1.6.1
     hooks:
       - id: mypy
+        additional_dependencies: [
+          "elasticsearch[async]==8.6.1",
+          "elastic-transport==8.4.1",
+          "types-tabulate",
+        ]
         args: [
+          "--ignore-missing-imports",
           "--config",
           "pyproject.toml"
         ]

esrally/client/asynchronous.py

@@ -77,7 +77,7 @@ async def send(self, conn: "Connection") -> "ClientResponse":
         self.response = self.response_class(
             self.method,
             self.original_url,
-            writer=self._writer,
+            writer=self._writer,  # type: ignore[arg-type]  # TODO remove this ignore when introducing type hints
             continue100=self._continue,
             timer=self._timer,
             request_info=self.request_info,
@@ -223,7 +223,7 @@ def __init__(self, config):
         self._loop = None
         self.client_id = None
         self.trace_configs = None
-        self.enable_cleanup_closed = None
+        self.enable_cleanup_closed = False
         self._static_responses = None
         self._request_class = aiohttp.ClientRequest
         self._response_class = aiohttp.ClientResponse

esrally/mechanic/team.py

@@ -20,7 +20,7 @@
 import os
 from enum import Enum
 from types import ModuleType
-from typing import Any, Collection, Mapping, Union
+from typing import Any, Collection, Mapping, Optional, Union
 
 import tabulate
 
@@ -49,7 +49,7 @@ def list_cars(cfg: types.Config):
     console.println(tabulate.tabulate([[c.name, c.type, c.description] for c in cars], headers=["Name", "Type", "Description"]))
 
 
-def load_car(repo: str, name: Collection[str], car_params: Mapping = None) -> "Car":
+def load_car(repo: str, name: Collection[str], car_params: Optional[Mapping] = None) -> "Car":
     class Component:
         def __init__(self, root_path, entry_point):
             self.root_path = root_path
@@ -165,7 +165,7 @@ def load_car(self, name, car_params=None):
         config = self._config_loader(car_config_file)
         root_paths = []
         config_paths = []
-        config_base_vars = {}
+        config_base_vars: Mapping[str, Any] = {}
         description = self._value(config, ["meta", "description"], default="")
         car_type = self._value(config, ["meta", "type"], default="car")
         config_bases = self._value(config, ["config", "base"], default="").split(",")
@@ -192,7 +192,7 @@ def load_car(self, name, car_params=None):
     def _config_loader(self, file_name):
         config = configparser.ConfigParser(interpolation=configparser.ExtendedInterpolation())
         # Do not modify the case of option keys but read them as is
-        config.optionxform = lambda option: option
+        config.optionxform = lambda optionstr: optionstr  # type: ignore[method-assign]
         config.read(file_name)
         return config
 
@@ -239,7 +239,7 @@ def __init__(
         names: Collection[str],
         root_path: Union[None, str, Collection[str]],
         config_paths: Collection[str],
-        variables: Mapping[str, Any] = None,
+        variables: Optional[Mapping[str, Any]] = None,
     ):
         """
         Creates new settings for a benchmark candidate.
@@ -252,12 +252,12 @@ def __init__(
         if variables is None:
             variables = {}
         if isinstance(names, str):
-            self.names = [names]
+            self.names: Collection[str] = [names]
         else:
             self.names = names
 
         if root_path is None:
-            self.root_path = []
+            self.root_path: Collection[str] = []
         elif isinstance(root_path, str):
             self.root_path = [root_path]
         else:
@@ -393,7 +393,7 @@ def load_plugin(self, name, config_names, plugin_params=None):
 
                 config = configparser.ConfigParser(interpolation=configparser.ExtendedInterpolation())
                 # Do not modify the case of option keys but read them as is
-                config.optionxform = lambda option: option
+                config.optionxform = lambda optionstr: optionstr  # type: ignore[method-assign]
                 config.read(config_file)
                 if "config" in config and "base" in config["config"]:
                     config_bases = config["config"]["base"].split(",")

esrally/utils/modules.py

@@ -15,7 +15,7 @@
 # specific language governing permissions and limitations
 # under the License.
 
-import importlib.machinery
+import importlib.util
 import logging
 import os
 import sys
@@ -45,7 +45,7 @@ def __init__(self, root_path: Union[str, Collection[str]], component_entry_point
         ``root_path``.
         :param recurse: Search recursively for modules but ignore modules starting with "_" (Default: ``True``).
         """
-        self.root_path: Collection[str] = root_path if isinstance(root_path, list) else [root_path]
+        self.root_path: Collection[str] = root_path if isinstance(root_path, list) else [str(root_path)]
         self.component_entry_point = component_entry_point
         self.recurse = recurse
         self.logger = logging.getLogger(__name__)
@@ -67,7 +67,7 @@ def _load_component(self, component_name: str, module_dirs: Collection[str], roo
         for name, p in self._modules(module_dirs, component_name, root_path):
             self.logger.debug("Loading module [%s]: %s", name, p)
             spec = importlib.util.spec_from_file_location(name, p)
-            if spec is None:
+            if spec is None or spec.loader is None:
                 raise exceptions.SystemSetupError(f"Could not load module [{name}]")
             m = importlib.util.module_from_spec(spec)
             spec.loader.exec_module(m)

pyproject.toml

@@ -143,12 +143,25 @@ junit_logging = "all"
 asyncio_mode = "strict"
 xfail_strict = true
 
-# With rare exceptions, Rally does not use type hints. The intention of the
-# following largely reduced mypy configuration scope is verification of argument
-# types in config.Config methods while introducing configuration properties
-# (props). The error we are after here is "arg-type".
+# With exceptions specified in mypy override section, Rally does not use type
+# hints (they were a novelty when Rally came to be). Hints are being slowly and
+# opportunistically introduced whenever we revisit a group of modules.
+#
+# The intention of the following largely reduced global config scope is
+# verification of argument types in config.Config methods while introducing
+# configuration properties (props). The intention of "disable_error_code" option
+# is to keep "arg-type" error code, while disabling other error codes.
+# Ref: https://github.com/elastic/rally/pull/1798
 [tool.mypy]
-python_version = 3.8
+python_version = "3.8"
+# subset of "strict", kept at global config level as some of the options are
+# supported only at this level
+# https://mypy.readthedocs.io/en/stable/existing_code.html#introduce-stricter-options
+warn_unused_configs = true
+warn_redundant_casts = true
+warn_unused_ignores = true
+strict_equality = true
+extra_checks = true
 check_untyped_defs = true
 disable_error_code = [
     "assignment",
@@ -168,3 +181,33 @@ disable_error_code = [
     "union-attr",
     "var-annotated",
 ]
+files = [
+    "esrally/",
+    "it/",
+    "tests/",
+]
+
+[[tool.mypy.overrides]]
+module = [
+    "esrally.mechanic.team",
+    "esrally.utils.modules",
+]
+# this should be a copy of disabled_error_code from above
+enable_error_code = [
+    "assignment",
+    "attr-defined",
+    "call-arg",
+    "call-overload",
+    "dict-item",
+    "import-not-found",
+    "import-untyped",
+    "index",
+    "list-item",
+    "misc",
+    "name-defined",
+    "operator",
+    "str-bytes-safe",
+    "syntax",
+    "union-attr",
+    "var-annotated",
+]

tests/client/factory_test.py

@@ -29,7 +29,7 @@
 import pytest
 import trustme
 import urllib3.exceptions
-from elastic_transport import ApiResponseMeta
+from elastic_transport import ApiResponseMeta, HttpHeaders, NodeConfig
 from pytest_httpserver import HTTPServer
 
 from esrally import client, doc_link, exceptions
@@ -38,7 +38,17 @@
 
 
 def _api_error(status, message):
-    return elasticsearch.ApiError(message, ApiResponseMeta(status=status, http_version="1.1", headers={}, duration=0.0, node=None), None)
+    return elasticsearch.ApiError(
+        message,
+        ApiResponseMeta(
+            status=status,
+            http_version="1.1",
+            headers=HttpHeaders(),
+            duration=0.0,
+            node=NodeConfig(scheme="https", host="localhost", port=9200),
+        ),
+        None,
+    )
 
 
 class TestEsClientFactory:
@@ -518,7 +528,7 @@ def test_connection_ssl_error(self, es):
     def test_connection_protocol_error(self, es):
         es.cluster.health.side_effect = elasticsearch.ConnectionError(
             message="N/A",
-            errors=[urllib3.exceptions.ProtocolError("Connection aborted.")],
+            errors=[urllib3.exceptions.ProtocolError("Connection aborted.")],  # type: ignore[arg-type]
         )
         with pytest.raises(
             exceptions.SystemSetupError,

tests/driver/driver_test.py

@@ -1894,7 +1894,13 @@ async def test_execute_single_with_connection_error_always_aborts(self, on_error
     async def test_execute_single_with_http_400_aborts_when_specified(self):
         es = None
         params = None
-        error_meta = elastic_transport.ApiResponseMeta(status=404, http_version="1.1", headers={}, duration=0.0, node=None)
+        error_meta = elastic_transport.ApiResponseMeta(
+            status=404,
+            http_version="1.1",
+            headers=elastic_transport.HttpHeaders(),
+            duration=0.0,
+            node=elastic_transport.NodeConfig(scheme="http", host="localhost", port=9200),
+        )
         runner = mock.AsyncMock(
             side_effect=elasticsearch.NotFoundError(message="not found", meta=error_meta, body="the requested document could not be found")
         )
@@ -1912,7 +1918,13 @@ async def test_execute_single_with_http_400_with_empty_raw_response_body(self):
         params = None
         empty_body = io.BytesIO(b"")
         str_literal_empty_body = str(empty_body)
-        error_meta = elastic_transport.ApiResponseMeta(status=413, http_version="1.1", headers={}, duration=0.0, node=None)
+        error_meta = elastic_transport.ApiResponseMeta(
+            status=413,
+            http_version="1.1",
+            headers=elastic_transport.HttpHeaders(),
+            duration=0.0,
+            node=elastic_transport.NodeConfig(scheme="http", host="localhost", port=9200),
+        )
         runner = mock.AsyncMock(side_effect=elasticsearch.ApiError(message=str_literal_empty_body, meta=error_meta, body=empty_body))
 
         with pytest.raises(exceptions.RallyAssertionError) as exc:
@@ -1925,7 +1937,13 @@ async def test_execute_single_with_http_400_with_raw_response_body(self):
         params = None
         body = io.BytesIO(b"Huge error")
         str_literal = str(body)
-        error_meta = elastic_transport.ApiResponseMeta(status=499, http_version="1.1", headers={}, duration=0.0, node=None)
+        error_meta = elastic_transport.ApiResponseMeta(
+            status=499,
+            http_version="1.1",
+            headers=elastic_transport.HttpHeaders(),
+            duration=0.0,
+            node=elastic_transport.NodeConfig(scheme="http", host="localhost", port=9200),
+        )
         runner = mock.AsyncMock(side_effect=elasticsearch.ApiError(message=str_literal, meta=error_meta, body=body))
 
         with pytest.raises(exceptions.RallyAssertionError) as exc:
@@ -1936,7 +1954,13 @@ async def test_execute_single_with_http_400_with_raw_response_body(self):
     async def test_execute_single_with_http_400(self):
         es = None
         params = None
-        error_meta = elastic_transport.ApiResponseMeta(status=404, http_version="1.1", headers={}, duration=0.0, node=None)
+        error_meta = elastic_transport.ApiResponseMeta(
+            status=404,
+            http_version="1.1",
+            headers=elastic_transport.HttpHeaders(),
+            duration=0.0,
+            node=elastic_transport.NodeConfig(scheme="http", host="localhost", port=9200),
+        )
         runner = mock.AsyncMock(
             side_effect=elasticsearch.NotFoundError(message="not found", meta=error_meta, body="the requested document could not be found")
         )
@@ -1956,7 +1980,13 @@ async def test_execute_single_with_http_400(self):
     async def test_execute_single_with_http_413(self):
         es = None
         params = None
-        error_meta = elastic_transport.ApiResponseMeta(status=413, http_version="1.1", headers={}, duration=0.0, node=None)
+        error_meta = elastic_transport.ApiResponseMeta(
+            status=413,
+            http_version="1.1",
+            headers=elastic_transport.HttpHeaders(),
+            duration=0.0,
+            node=elastic_transport.NodeConfig(scheme="http", host="localhost", port=9200),
+        )
         runner = mock.AsyncMock(side_effect=elasticsearch.NotFoundError(message="", meta=error_meta, body=""))
 
         ops, unit, request_meta_data = await driver.execute_single(self.context_managed(runner), es, params, on_error="continue")