chore: make pyright run in strict mode and add mypy (#78)

* ci: add mypy

This commit introduces mypy to our pre-commit hooks and CI. This should
increase the amount of typing bugs we catch.

* refactor: make pyright and mypy strict

This commit makes both pyright and mypy operate on the strictest
setting. I had to change some code around or add type ignores in cases
where it's being too strict or when the stubs for dependencies are not
precise enough.

I only did this for the `dbtsl` package in this commit. I'll refactor
the tests in the next commit.

* test: pyright in tests

This commit changes some of our tests to work with stricter pyright
settings. I made the checks for tests a little less strict than the real
`dbtsl` tests since our tests sometimes inject wrong types into the API
to purposefully induce errors, or mocks stuff that's really hard to
type.

* docs: changelog

Author: serramatutu

.changes/unreleased/Chore-20250403-163034.yaml

@@ -0,0 +1,3 @@
+kind: Chore
+body: Make `pyright` run on `strict` mode and add `mypy`.
+time: 2025-04-03T16:30:34.024926+02:00

.github/workflows/code-quality.yaml

@@ -29,7 +29,10 @@ jobs:
         run: "hatch run dev:ruff format --check"
 
       - name: basedpyright
-        run: "hatch run dev:basedpyright"
+        run: "hatch run dev:basedpyright dbtsl tests"
+
+      - name: mypy
+        run: "hatch run dev:python -m mypy dbtsl"
 
       - name: fetch server schema
         run: "hatch run dev:fetch-schema"

dbtsl/__init__.py

@@ -1,11 +1,11 @@
-# pyright: reportUnusedImport=false
+# type: ignore
 try:
     from dbtsl.client.sync import SyncSemanticLayerClient
 
     SemanticLayerClient = SyncSemanticLayerClient
 except ImportError:
 
-    def err_factory(*args, **kwargs) -> None:  # noqa: D103
+    def err_factory(*_args: object, **_kwargs: object) -> None:  # noqa: D103
         raise ImportError(
             "You are trying to use the default `SemanticLayerClient`, "
             "but it looks like the necessary dependencies were not installed. "

dbtsl/api/adbc/client/asyncio.py

@@ -6,8 +6,7 @@
 from typing_extensions import Self, Unpack
 
 from dbtsl.api.adbc.client.base import BaseADBCClient
-from dbtsl.api.adbc.protocol import QueryParameters
-from dbtsl.api.shared.query_params import DimensionValuesQueryParameters
+from dbtsl.api.shared.query_params import DimensionValuesQueryParameters, QueryParameters
 
 
 class AsyncADBCClient(BaseADBCClient):
@@ -59,7 +58,7 @@ async def query(self, **query_params: Unpack[QueryParameters]) -> pa.Table:
         # just creating the cursor object doesn't perform any blocking IO.
         with self._conn.cursor() as cur:
             try:
-                await self._loop.run_in_executor(None, cur.execute, query_sql)
+                await self._loop.run_in_executor(None, cur.execute, query_sql)  # pyright: ignore[reportUnknownArgumentType,reportUnknownMemberType]
             except Exception as err:
                 self._handle_error(err)
             table = await self._loop.run_in_executor(None, cur.fetch_arrow_table)
@@ -74,7 +73,8 @@ async def dimension_values(self, **query_params: Unpack[DimensionValuesQueryPara
         # just creating the cursor object doesn't perform any blocking IO.
         with self._conn.cursor() as cur:
             try:
-                await self._loop.run_in_executor(None, cur.execute, query_sql)
+                await self._loop.run_in_executor(None, cur.execute, query_sql)  # pyright: ignore[reportUnknownArgumentType,reportUnknownMemberType]
+
             except Exception as err:
                 self._handle_error(err)
             table = await self._loop.run_in_executor(None, cur.fetch_arrow_table)

dbtsl/api/adbc/client/base.py

@@ -4,7 +4,7 @@
 
 from adbc_driver_flightsql import DatabaseOptions
 from adbc_driver_flightsql.dbapi import Connection
-from adbc_driver_flightsql.dbapi import connect as adbc_connect
+from adbc_driver_flightsql.dbapi import connect as adbc_connect  # pyright: ignore[reportUnknownVariableType]
 from adbc_driver_manager import AdbcStatusCode, ProgrammingError
 
 import dbtsl.env as env

dbtsl/api/adbc/client/sync.py

@@ -5,8 +5,7 @@
 from typing_extensions import Self, Unpack
 
 from dbtsl.api.adbc.client.base import BaseADBCClient
-from dbtsl.api.adbc.protocol import QueryParameters
-from dbtsl.api.shared.query_params import DimensionValuesQueryParameters
+from dbtsl.api.shared.query_params import DimensionValuesQueryParameters, QueryParameters
 
 
 class SyncADBCClient(BaseADBCClient):
@@ -53,7 +52,7 @@ def query(self, **query_params: Unpack[QueryParameters]) -> pa.Table:
 
         with self._conn.cursor() as cur:
             try:
-                cur.execute(query_sql)
+                cur.execute(query_sql)  # pyright: ignore[reportUnknownMemberType]
             except Exception as err:
                 self._handle_error(err)
 
@@ -67,7 +66,7 @@ def dimension_values(self, **query_params: Unpack[DimensionValuesQueryParameters
 
         with self._conn.cursor() as cur:
             try:
-                cur.execute(query_sql)
+                cur.execute(query_sql)  # pyright: ignore[reportUnknownMemberType]
             except Exception as err:
                 self._handle_error(err)
 

dbtsl/api/adbc/protocol.py

@@ -20,7 +20,7 @@ def _serialize_val(cls, val: Any) -> str:
             return str(val)
 
         if isinstance(val, list):
-            list_str = ",".join(cls._serialize_val(list_val) for list_val in val)
+            list_str = ",".join(cls._serialize_val(list_val) for list_val in val)  # pyright: ignore[reportUnknownVariableType]
             return f"[{list_str}]"
 
         if isinstance(val, OrderByMetric):

dbtsl/api/graphql/client/asyncio.py

@@ -21,7 +21,7 @@
 from dbtsl.api.shared.query_params import QueryParameters
 from dbtsl.backoff import ExponentialBackoff
 from dbtsl.error import ConnectTimeoutError, ExecuteTimeoutError, QueryFailedError, RetryTimeoutError, TimeoutError
-from dbtsl.models.query import QueryId, QueryStatus
+from dbtsl.models.query import QueryStatus
 
 # aiohttp only started distinguishing between read and connect timeouts after version 3.10
 # If the user is using an older version, we fall back to considering them both the same thing
@@ -30,13 +30,13 @@
 
     AiohttpServerTimeout = ServerTimeoutError
     AiohttpConnectionTimeout = ConnectionTimeoutError
-    NEW_AIOHTTP = True
+    _new_aiohttp = True
 except ImportError:
     from asyncio import TimeoutError as AsyncioTimeoutError
 
     AiohttpServerTimeout = AsyncioTimeoutError
     AiohttpConnectionTimeout = AsyncioTimeoutError
-    NEW_AIOHTTP = False
+    _new_aiohttp = False
 
 
 class AsyncGraphQLClient(BaseGraphQLClient[AIOHTTPTransport, AsyncClientSession]):
@@ -75,7 +75,7 @@ def _create_transport(self, url: str, headers: Dict[str, str]) -> AIOHTTPTranspo
             # The following type ignore is OK since gql annotated `timeout` as an `Optional[int]`,
             # but aiohttp allows `float` timeouts
             # See: https://docs.aiohttp.org/en/stable/client_reference.html#aiohttp.ClientTimeout
-            timeout=self.timeout.execute_timeout,  # pyright: ignore[reportArgumentType]
+            timeout=self.timeout.execute_timeout,  # type: ignore
             ssl_close_timeout=self.timeout.tls_close_timeout,
         )
 
@@ -95,21 +95,21 @@ async def session(self) -> AsyncIterator[Self]:
             yield self
             self._gql_session_unsafe = None
 
-    async def _run(self, op: ProtocolOperation[TVariables, TResponse], **kwargs: TVariables) -> TResponse:
+    async def _run(self, op: ProtocolOperation[TVariables, TResponse], raw_variables: TVariables) -> TResponse:
         """Run a `ProtocolOperation`."""
         raw_query = op.get_request_text()
-        variables = op.get_request_variables(environment_id=self.environment_id, **kwargs)
+        variables = op.get_request_variables(environment_id=self.environment_id, variables=raw_variables)
         gql_query = gql(raw_query)
 
         try:
-            res = await self._gql_session.execute(gql_query, variable_values=variables)
+            res = await self._gql_session.execute(gql_query, variable_values=variables)  # type: ignore
         except AiohttpConnectionTimeout as err:
-            if NEW_AIOHTTP:
+            if _new_aiohttp:
                 raise ConnectTimeoutError(timeout_s=self.timeout.connect_timeout) from err
             raise TimeoutError(timeout_s=self.timeout.total_timeout) from err
         # I found out by trial and error that aiohttp can raise all these different kinds of errors
         # depending on where the timeout happened in the stack (aiohttp, anyio, asyncio)
-        except (AiohttpServerTimeout, asyncio.TimeoutError, BuiltinTimeoutError) as err:
+        except (AiohttpServerTimeout, asyncio.TimeoutError, BuiltinTimeoutError) as err:  # type: ignore
             raise ExecuteTimeoutError(timeout_s=self.timeout.execute_timeout) from err
         except Exception as err:
             raise self._refine_err(err)
@@ -118,10 +118,9 @@ async def _run(self, op: ProtocolOperation[TVariables, TResponse], **kwargs: TVa
 
     async def _poll_until_complete(
         self,
-        query_id: QueryId,
         poll_op: ProtocolOperation[TJobStatusVariables, TJobStatusResult],
+        variables: TJobStatusVariables,
         backoff: Optional[ExponentialBackoff] = None,
-        **kwargs,
     ) -> TJobStatusResult:
         """Poll for a job's results until it is in a completed state (SUCCESSFUL or FAILED)."""
         if backoff is None:
@@ -132,8 +131,7 @@ async def _poll_until_complete(
 
         start_s = time.time()
         for sleep_ms in backoff.iter_ms():
-            kwargs["query_id"] = query_id
-            qr = await self._run(poll_op, **kwargs)
+            qr = await self._run(op=poll_op, raw_variables=variables)
             if qr.status in (QueryStatus.SUCCESSFUL, QueryStatus.FAILED):
                 return qr
 
@@ -149,7 +147,10 @@ async def _poll_until_complete(
     async def query(self, **params: Unpack[QueryParameters]) -> "pa.Table":
         """Query the Semantic Layer."""
         query_id = await self.create_query(**params)
-        first_page_results = await self._poll_until_complete(query_id, self.PROTOCOL.get_query_result, page_num=1)
+        first_page_results = await self._poll_until_complete(
+            poll_op=self.PROTOCOL.get_query_result,
+            variables={"query_id": query_id, "page_num": 1},
+        )
         if first_page_results.status != QueryStatus.SUCCESSFUL:
             raise QueryFailedError()
 
@@ -164,5 +165,5 @@ async def query(self, **params: Unpack[QueryParameters]) -> "pa.Table":
         ]
         all_page_results = [first_page_results] + await asyncio.gather(*tasks)
         tables = [r.result_table for r in all_page_results]
-        final_table = pa.concat_tables(tables)
+        final_table = pa.concat_tables(tables)  # type: ignore
         return final_table

dbtsl/api/graphql/client/asyncio.pyi

@@ -1,3 +1,5 @@
+# mypy: disable-error-code="misc"
+
 from contextlib import AbstractAsyncContextManager
 from typing import List, Optional, Self, Union
 

dbtsl/api/graphql/client/base.py

@@ -1,4 +1,3 @@
-import functools
 import warnings
 from abc import abstractmethod
 from typing import Any, Dict, Generic, Optional, Protocol, TypeVar, Union
@@ -126,13 +125,15 @@ def __getattr__(self, attr: str) -> Any:
 
         with warnings.catch_warnings():
             warnings.simplefilter("ignore", DeprecationWarning)
-            return functools.partial(
-                self._run,
-                op=op,
-            )
+
+            def wrapped(**kwargs: Any) -> Any:
+                return self._run(op=op, raw_variables=kwargs)
+
+            return wrapped
 
 
-TClient = TypeVar("TClient", bound=BaseGraphQLClient, covariant=True)
+# TODO: have to type ignore, see: https://github.com/microsoft/pyright/issues/3497
+TClient = TypeVar("TClient", bound=BaseGraphQLClient, covariant=True)  # type: ignore
 
 
 class GraphQLClientFactory(Protocol, Generic[TClient]):  # noqa: D101

dbtsl/api/graphql/client/sync.py

@@ -25,7 +25,7 @@
 from dbtsl.api.shared.query_params import QueryParameters
 from dbtsl.backoff import ExponentialBackoff
 from dbtsl.error import ConnectTimeoutError, ExecuteTimeoutError, QueryFailedError, RetryTimeoutError
-from dbtsl.models.query import QueryId, QueryStatus
+from dbtsl.models.query import QueryStatus
 
 
 class SyncGraphQLClient(BaseGraphQLClient[RequestsHTTPTransport, SyncClientSession]):
@@ -64,7 +64,7 @@ def _create_transport(self, url: str, headers: Dict[str, str]) -> RequestsHTTPTr
             # but requests allows `tuple[float, float]` timeouts
             # See: https://github.com/graphql-python/gql/blob/b066e8944b0da0a4bbac6c31f43e5c3c7772cd51/gql/transport/requests.py#L393
             # See: https://requests.readthedocs.io/en/latest/user/advanced/#timeouts
-            timeout=(self.timeout.connect_timeout, self.timeout.execute_timeout),  # pyright: ignore[reportArgumentType]
+            timeout=(self.timeout.connect_timeout, self.timeout.execute_timeout),  # type: ignore
         )
 
     @contextmanager
@@ -83,14 +83,14 @@ def session(self) -> Iterator[Self]:
             yield self
             self._gql_session_unsafe = None
 
-    def _run(self, op: ProtocolOperation[TVariables, TResponse], **kwargs: TVariables) -> TResponse:
+    def _run(self, op: ProtocolOperation[TVariables, TResponse], raw_variables: TVariables) -> TResponse:
         """Run a `ProtocolOperation`."""
         raw_query = op.get_request_text()
-        variables = op.get_request_variables(environment_id=self.environment_id, **kwargs)
+        variables = op.get_request_variables(environment_id=self.environment_id, variables=raw_variables)
         gql_query = gql(raw_query)
 
         try:
-            res = self._gql_session.execute(gql_query, variable_values=variables)
+            res = self._gql_session.execute(gql_query, variable_values=variables)  # type: ignore
         except RequestsReadTimeout as err:
             raise ExecuteTimeoutError(timeout_s=self.timeout.execute_timeout) from err
         except RequestsConnectTimeout as err:
@@ -102,10 +102,9 @@ def _run(self, op: ProtocolOperation[TVariables, TResponse], **kwargs: TVariable
 
     def _poll_until_complete(
         self,
-        query_id: QueryId,
         poll_op: ProtocolOperation[TJobStatusVariables, TJobStatusResult],
+        variables: TJobStatusVariables,
         backoff: Optional[ExponentialBackoff] = None,
-        **kwargs,
     ) -> TJobStatusResult:
         """Poll for a query's results until it is in a completed state (SUCCESSFUL or FAILED).
 
@@ -120,9 +119,7 @@ def _poll_until_complete(
 
         start_s = time.time()
         for sleep_ms in backoff.iter_ms():
-            kwargs["query_id"] = query_id
-
-            qr = self._run(poll_op, **kwargs)
+            qr = self._run(op=poll_op, raw_variables=variables)
             if qr.status in (QueryStatus.SUCCESSFUL, QueryStatus.FAILED):
                 return qr
 
@@ -138,7 +135,13 @@ def _poll_until_complete(
     def query(self, **params: Unpack[QueryParameters]) -> "pa.Table":
         """Query the Semantic Layer."""
         query_id = self.create_query(**params)
-        first_page_results = self._poll_until_complete(query_id, self.PROTOCOL.get_query_result, page_num=1)
+        first_page_results = self._poll_until_complete(
+            poll_op=self.PROTOCOL.get_query_result,
+            variables={
+                "query_id": query_id,
+                "page_num": 1,
+            },
+        )
         if first_page_results.status != QueryStatus.SUCCESSFUL:
             raise QueryFailedError()
 
@@ -153,5 +156,5 @@ def query(self, **params: Unpack[QueryParameters]) -> "pa.Table":
         ]
         all_page_results = [first_page_results] + results
         tables = [r.result_table for r in all_page_results]
-        final_table = pa.concat_tables(tables)
+        final_table = pa.concat_tables(tables)  # type: ignore
         return final_table

dbtsl/api/graphql/client/sync.pyi

@@ -1,3 +1,5 @@
+# mypy: disable-error-code="misc"
+
 from contextlib import AbstractContextManager
 from typing import Iterator, List, Optional, Union