Merge pull request #245 from tisnik/lcore-303-fixed-issues-found-by-pyright

LCORE-303: fixed issues found by pyright

Author: tisnik

.github/workflows/pylint.yaml

@@ -20,4 +20,4 @@ jobs:
       - name: Install dependencies
         run: uv sync
       - name: Python linter
-        run: uv run pylint src
+        run: uv run pylint src tests

Makefile

@@ -60,7 +60,7 @@ black:
 	uv run black --check .
 
 pylint:
-	uv run pylint src
+	uv run pylint src tests
 
 pyright:
 	uv run pyright src

tests/unit/app/endpoints/test_models.py

@@ -4,7 +4,7 @@
 
 import pytest
 
-from fastapi import HTTPException, status
+from fastapi import HTTPException, Request, status
 
 from app.endpoints.models import models_endpoint_handler
 from configuration import AppConfig
@@ -19,7 +19,13 @@ def test_models_endpoint_handler_configuration_not_loaded(mocker):
     )
     mocker.patch("app.endpoints.models.configuration", None)
 
-    request = None
+    request = Request(
+        scope={
+            "type": "http",
+            "headers": [(b"authorization", b"Bearer invalid-token")],
+        }
+    )
+
     with pytest.raises(HTTPException) as e:
         models_endpoint_handler(request)
         assert e.status_code == status.HTTP_500_INTERNAL_SERVER_ERROR
@@ -58,7 +64,12 @@ def test_models_endpoint_handler_improper_llama_stack_configuration(mocker):
         return_value=None,
     )
 
-    request = None
+    request = Request(
+        scope={
+            "type": "http",
+            "headers": [(b"authorization", b"Bearer invalid-token")],
+        }
+    )
     with pytest.raises(HTTPException) as e:
         models_endpoint_handler(request)
         assert e.status_code == status.HTTP_500_INTERNAL_SERVER_ERROR
@@ -91,8 +102,14 @@ def test_models_endpoint_handler_configuration_loaded():
     cfg = AppConfig()
     cfg.init_from_dict(config_dict)
 
+    request = Request(
+        scope={
+            "type": "http",
+            "headers": [(b"authorization", b"Bearer invalid-token")],
+        }
+    )
+
     with pytest.raises(HTTPException) as e:
-        request = None
         models_endpoint_handler(request)
         assert e.status_code == status.HTTP_500_INTERNAL_SERVER_ERROR
         assert e.detail["response"] == "Unable to connect to Llama Stack"
@@ -132,6 +149,11 @@ def test_models_endpoint_handler_unable_to_retrieve_models_list(mocker):
     mock_config = mocker.Mock()
     mocker.patch("app.endpoints.models.configuration", mock_config)
 
-    request = None
+    request = Request(
+        scope={
+            "type": "http",
+            "headers": [(b"authorization", b"Bearer invalid-token")],
+        }
+    )
     response = models_endpoint_handler(request)
     assert response is not None

tests/unit/auth/test_auth.py

@@ -8,20 +8,23 @@
 
 def test_get_auth_dependency_noop():
     """Test getting Noop authentication dependency."""
+    assert configuration.authentication_configuration is not None
     configuration.authentication_configuration.module = AUTH_MOD_NOOP
     auth_dependency = get_auth_dependency()
     assert isinstance(auth_dependency, noop.NoopAuthDependency)
 
 
 def test_get_auth_dependency_noop_with_token():
     """Test getting Noop with token authentication dependency."""
+    assert configuration.authentication_configuration is not None
     configuration.authentication_configuration.module = AUTH_MOD_NOOP_WITH_TOKEN
     auth_dependency = get_auth_dependency()
     assert isinstance(auth_dependency, noop_with_token.NoopWithTokenAuthDependency)
 
 
 def test_get_auth_dependency_k8s():
     """Test getting K8s authentication dependency."""
+    assert configuration.authentication_configuration is not None
     configuration.authentication_configuration.module = AUTH_MOD_K8S
     auth_dependency = get_auth_dependency()
     assert isinstance(auth_dependency, k8s.K8SAuthDependency)

tests/unit/auth/test_utils.py

@@ -1,19 +1,21 @@
 """Unit tests for functions defined in auth/utils.py"""
 
 from fastapi import HTTPException
+from starlette.datastructures import Headers
+
 from auth.utils import extract_user_token
 
 
 def test_extract_user_token():
     """Test extracting user token from headers."""
-    headers = {"Authorization": "Bearer abcdef123"}
+    headers = Headers({"Authorization": "Bearer abcdef123"})
     token = extract_user_token(headers)
     assert token == "abcdef123"
 
 
 def test_extract_user_token_no_header():
     """Test extracting user token when no Authorization header is present."""
-    headers = {}
+    headers = Headers({})
     try:
         extract_user_token(headers)
     except HTTPException as exc:
@@ -23,7 +25,7 @@ def test_extract_user_token_no_header():
 
 def test_extract_user_token_invalid_format():
     """Test extracting user token with invalid Authorization header format."""
-    headers = {"Authorization": "InvalidFormat"}
+    headers = Headers({"Authorization": "InvalidFormat"})
     try:
         extract_user_token(headers)
     except HTTPException as exc:

tests/unit/models/test_config.py

@@ -187,9 +187,9 @@ def test_user_data_collection_data_collector_wrong_configuration() -> None:
 def test_tls_configuration() -> None:
     """Test the TLS configuration."""
     cfg = TLSConfiguration(
-        tls_certificate_path="tests/configuration/server.crt",
-        tls_key_path="tests/configuration/server.key",
-        tls_key_password="tests/configuration/password",
+        tls_certificate_path=Path("tests/configuration/server.crt"),
+        tls_key_path=Path("tests/configuration/server.key"),
+        tls_key_password=Path("tests/configuration/password"),
     )
     assert cfg is not None
     assert cfg.tls_certificate_path == Path("tests/configuration/server.crt")
@@ -201,59 +201,59 @@ def test_tls_configuration_wrong_certificate_path() -> None:
     """Test the TLS configuration loading when some path is broken."""
     with pytest.raises(ValueError, match="Path does not point to a file"):
         TLSConfiguration(
-            tls_certificate_path="this-is-wrong",
-            tls_key_path="tests/configuration/server.key",
-            tls_key_password="tests/configuration/password",
+            tls_certificate_path=Path("this-is-wrong"),
+            tls_key_path=Path("tests/configuration/server.key"),
+            tls_key_password=Path("tests/configuration/password"),
         )
 
 
 def test_tls_configuration_wrong_key_path() -> None:
     """Test the TLS configuration loading when some path is broken."""
     with pytest.raises(ValueError, match="Path does not point to a file"):
         TLSConfiguration(
-            tls_certificate_path="tests/configurationserver.crt",
-            tls_key_path="this-is-wrong",
-            tls_key_password="tests/configuration/password",
+            tls_certificate_path=Path("tests/configurationserver.crt"),
+            tls_key_path=Path("this-is-wrong"),
+            tls_key_password=Path("tests/configuration/password"),
         )
 
 
 def test_tls_configuration_wrong_password_path() -> None:
     """Test the TLS configuration loading when some path is broken."""
     with pytest.raises(ValueError, match="Path does not point to a file"):
         TLSConfiguration(
-            tls_certificate_path="tests/configurationserver.crt",
-            tls_key_path="tests/configuration/server.key",
-            tls_key_password="this-is-wrong",
+            tls_certificate_path=Path("tests/configurationserver.crt"),
+            tls_key_path=Path("tests/configuration/server.key"),
+            tls_key_password=Path("this-is-wrong"),
         )
 
 
 def test_tls_configuration_certificate_path_to_directory() -> None:
     """Test the TLS configuration loading when some path points to a directory."""
     with pytest.raises(ValueError, match="Path does not point to a file"):
         TLSConfiguration(
-            tls_certificate_path="tests/",
-            tls_key_path="tests/configuration/server.key",
-            tls_key_password="tests/configuration/password",
+            tls_certificate_path=Path("tests/"),
+            tls_key_path=Path("tests/configuration/server.key"),
+            tls_key_password=Path("tests/configuration/password"),
         )
 
 
 def test_tls_configuration_key_path_to_directory() -> None:
     """Test the TLS configuration loading when some path points to a directory."""
     with pytest.raises(ValueError, match="Path does not point to a file"):
         TLSConfiguration(
-            tls_certificate_path="tests/configurationserver.crt",
-            tls_key_path="tests/",
-            tls_key_password="tests/configuration/password",
+            tls_certificate_path=Path("tests/configurationserver.crt"),
+            tls_key_path=Path("tests/"),
+            tls_key_password=Path("tests/configuration/password"),
         )
 
 
 def test_tls_configuration_password_path_to_directory() -> None:
     """Test the TLS configuration loading when some path points to a directory."""
     with pytest.raises(ValueError, match="Path does not point to a file"):
         TLSConfiguration(
-            tls_certificate_path="tests/configurationserver.crt",
-            tls_key_path="tests/configuration/server.key",
-            tls_key_password="tests/",
+            tls_certificate_path=Path("tests/configurationserver.crt"),
+            tls_key_path=Path("tests/configuration/server.key"),
+            tls_key_password=Path("tests/"),
         )
 
 
@@ -283,13 +283,13 @@ def test_model_context_protocol_server_required_fields() -> None:
     """Test that ModelContextProtocolServer requires name and url."""
 
     with pytest.raises(ValidationError):
-        ModelContextProtocolServer()
+        ModelContextProtocolServer()  # pyright: ignore
 
     with pytest.raises(ValidationError):
-        ModelContextProtocolServer(name="test-server")
+        ModelContextProtocolServer(name="test-server")  # pyright: ignore
 
     with pytest.raises(ValidationError):
-        ModelContextProtocolServer(url="http://localhost:8080")
+        ModelContextProtocolServer(url="http://localhost:8080")  # pyright: ignore
 
 
 def test_configuration_empty_mcp_servers() -> None:

tests/unit/models/test_responses.py

@@ -62,10 +62,12 @@ def test_constructor(self) -> None:
     def test_constructor_fields_required(self) -> None:
         """Test the AuthorizedResponse constructor."""
         with pytest.raises(ValidationError):
-            AuthorizedResponse(username="testuser")
+            AuthorizedResponse(username="testuser")  # pyright: ignore
 
         with pytest.raises(ValidationError):
-            AuthorizedResponse(user_id="123e4567-e89b-12d3-a456-426614174000")
+            AuthorizedResponse(
+                user_id="123e4567-e89b-12d3-a456-426614174000"
+            )  # pyright: ignore
 
 
 class TestUnauthorizedResponse:
@@ -81,4 +83,4 @@ def test_constructor(self) -> None:
     def test_constructor_fields_required(self) -> None:
         """Test the UnauthorizedResponse constructor."""
         with pytest.raises(Exception):
-            UnauthorizedResponse()
+            UnauthorizedResponse()  # pyright: ignore

tests/unit/utils/test_checks.py

@@ -1,6 +1,7 @@
 """Unit tests for functions defined in utils/checks module."""
 
 import os
+from pathlib import Path
 from unittest.mock import patch
 
 import pytest
@@ -69,7 +70,7 @@ def test_file_check_existing_file(input_file):
 def test_file_check_non_existing_file():
     """Test the function file_check for non existing file."""
     with pytest.raises(checks.InvalidConfigurationError):
-        checks.file_check("does-not-exists", "description")
+        checks.file_check(Path("does-not-exists"), "description")
 
 
 def test_file_check_not_readable_file(input_file):

tests/unit/utils/test_types.py

@@ -28,11 +28,13 @@ def test_get_tool_parser_when_model_id_starts_with_granite(self):
     def test_get_tool_calls_from_completion_message_when_none(self):
         """Test that get_tool_calls returns an empty array when CompletionMessage is None."""
         tool_parser = GraniteToolParser.get_parser("granite-3.3-8b-instruct")
+        assert tool_parser is not None, "tool parser was not returned"
         assert tool_parser.get_tool_calls(None) == [], "get_tool_calls should return []"
 
     def test_get_tool_calls_from_completion_message_when_not_none(self):
         """Test that get_tool_calls returns an empty array when CompletionMessage has no tool_calls."""  # pylint: disable=line-too-long
         tool_parser = GraniteToolParser.get_parser("granite-3.3-8b-instruct")
+        assert tool_parser is not None, "tool parser was not returned"
         completion_message = Mock()
         completion_message.tool_calls = []
         assert not tool_parser.get_tool_calls(
@@ -42,6 +44,7 @@ def test_get_tool_calls_from_completion_message_when_not_none(self):
     def test_get_tool_calls_from_completion_message_when_message_has_tool_calls(self):
         """Test that get_tool_calls returns the tool_calls when CompletionMessage has tool_calls."""
         tool_parser = GraniteToolParser.get_parser("granite-3.3-8b-instruct")
+        assert tool_parser is not None, "tool parser was not returned"
         completion_message = Mock()
         tool_calls = [Mock(tool_name="tool-1"), Mock(tool_name="tool-2")]
         completion_message.tool_calls = tool_calls