chore: type checking with pyright

[tromai]

This PR adds type checking with pyright.

The section below discusses the errors raised by pyright, and my proposed fixes.

ops.testing.SIMULATE_CAN_CONNECT not found

In tests/__init__.py

import ops.testing

ops.testing.SIMULATE_CAN_CONNECT = True

Solution: Remove ops.testing.SIMULATE_CAN_CONNECT = True following canonical/operator#873.

urllib.request not recognized

In both src/configchangesocket.py and src/controlsocket.py.

import urllib

...python
def __init__(self, socket_path: str, opener: urllib.request.OpenerDirector | None = None):
    super().__init__(socket_path, opener=opener)

This is a trivial fix.

- import urllib
+ import urllib.request

body[...] = value type mismatch

In controlsocket.py

body = { 
    "grpc_endpoint": grpc_endpoint,
    "http_endpoint": http_endpoint,
    "ca_cert": ca_cert,
}

...
if stack_traces is not None:
    body["stack_traces"] = stack_traces   # stack_traces is of type bool.

body is inferred as dict[str, str | None] when it is first initialized. However, bool/float values are later inserted to it.

Solution: annotate the type of body.

- body = { 
+ body: dict[str, str | bool | float | None] = {
    "grpc_endpoint": grpc_endpoint,
    "http_endpoint": http_endpoint,
    "ca_cert": ca_cert,
}

_current_open_telemetry_config returns type mismatch

In src/charm.py

def _current_open_telemetry_config(self) -> tuple[bool, float, str, bool]:
    sample_ratio = float(self.config["workload-tracing-sample-ratio"])
    self._validate_open_telemetry_sample_ratio(sample_ratio)
    return (
        self.config["workload-tracing-stack-traces"],
        sample_ratio,
        self.config["workload-tracing-tail-sampling-threshold"],
        self.config["workload-tracing-insecure-skip-verify"],
    )

Values of self.config (of type ConfigData) has a Union type bool | int | float | str. Therefore, pyright infers the returned object as:

tuple[ 
    bool | int | float | str,
    float,
    bool | int | float | str,
    bool | int | float | str
]

which doesn't match tuple[bool, float, str, bool] in the function signature.

Solution: use cast(T, ...)

def _current_open_telemetry_config(self) -> tuple[bool, float, str, bool]:
    sample_ratio = float(self.config["workload-tracing-sample-ratio"])
    self._validate_open_telemetry_sample_ratio(sample_ratio)
    return (
        cast(bool, self.config["workload-tracing-stack-traces"]),
        sample_ratio,
        cast(str, self.config["workload-tracing-tail-sampling-threshold"]),
        cast(bool, self.config["workload-tracing-insecure-skip-verify"]),
    )

hdrs=None type mismatch in urllib.error.HTTPError

In tests/test_sockets.py:

error=urllib.error.HTTPError(
    url="http://localhost/loki-endpoint",
    code=500,
    msg="",
    hdrs=None,
    fp=io.BytesIO(rb'{"error":"internal error"}'),
),

urllib.error.HTTPError expects hdrs: email.message.Message, not None. See here.

Solution: initialize an empty email.message.Message object.

test_sockets.py — MockOpener not assignable to OpenerDirector

A MockOpener object is passed into ControlSocketClient, for example:

def test_connection_error(self):
    mock_opener = MockOpener(self)
    control_socket = ControlSocketClient("fake_socket_path", opener=mock_opener)

The opener parameter of ControlSocketClient has type urllib.request.OpenerDirector | None = None. Therefore, pyright flags a non-compatible type.

This is an expected type error, but not a serious one. Since this is for testing purposes, and we control the behaviour of MockOpener, I proposed ignoring this error with # type: ignore.

---

Need close review

JujuControllerCharm.api_port should return int

parsed_url = urllib.parse.urlsplit("//" + api_addresses[0])
if not parsed_url.port:
    raise AgentConfException("API address does not include port")
return parsed_url.port

The .port attribute is an int if present, else it's None (see here). The guard if not parsed_url.port is not recommended since a 0 is falsy. I recommend guarding on the None value instead:

parsed_url = urllib.parse.urlsplit("//" + api_addresses[0])
if parsed_url.port is None:
    raise AgentConfException("API address does not include port")
return parsed_url.port

I also updated the type annotation of this function to int instead of str.

JujuControllerCharm.ca_cert return type annotation

    def ca_cert(self) -> str | None:
        """Return the controller's CA certificate."""
        ca_cert = self._controller_runtime_config("ca-cert")
        if ca_cert is None or not isinstance(ca_cert, str):
            return None

        return ca_cert

In theory, self._controller_runtime_config("ca-cert") can return None, or a value that is not of type str. I updated this function to strictly handle mis-match type. Other places in charm.py already handles ca_cert returning None correctly.

As a drive-by, I added JujuControllerCharm._controller_runtime_config return type since runtime_conf.get(key) returns None if key doesn't exist.

with open(runtime_conf_path) as runtime_conf_file:
    runtime_conf = yaml.safe_load(runtime_conf_file) or {}
    return runtime_conf.get(key)

[tromai]

@SimonRichardson This PR is ready for review. However, it is based on top of #141's branch (I performed a couple of merges). I suggest reviewing and merging #141 first before this one.