MyPy errors when using sync objects

[florianvazelle]

Which project are you reporting a bug for?

kr8s

What happened?

It seems that MyPy continues to interpret methods as asynchronous for sync objects, even though at runtime they have been converted to synchronous methods with the sync decorator. This leads to MyPy errors like:

$ mypy test.py 
test.py:3: error: Value of type "Coroutine[Any, Any, APIObject | list[APIObject]]" must be used  [unused-coroutine]
test.py:3: note: Are you missing an await?

with code like:

# test.py
from kr8s.objects import Pod
Pod.list()

Anything else?

No response

[jacobtomlinson]

Yeah I can reproduce that. I also see a similar error of I try and assign to a typed variable.

from kr8s.objects import Pod

pods: list[Pod] = Pod.list()

$ mypy test.py
test.py:3: error: Incompatible types in assignment (expression has type "Coroutine[Any, Any, APIObject | list[APIObject]]", variable has type "list[Pod]")  [assignment]
Found 1 error in 1 file (checked 1 source file)

I think there are two potential solutions here:

• Find some way to tell mypy that the @sync wrapper changes the type
  • We already do this in sphinx-autoapi via a plugin. Maybe there is a way to do this for mypy?
• Provide a .pyi stub file for each sync API submodule to override the typing
  • I would want to find a way to add tests to ensure this stays in sync. Otherwise we just introduce a new class of bug where the stubs get out of sync with the code. I had a quick play around with stubgen and stubtest but I'm not sure if they will run into similar issues as mypy around incorrectly identifying the signatures of wrapped classes.

[jacobtomlinson]

A third option could be to do away with the @sync wrapper and manually create each method and use run_sync() directly. This is what we do for functions like kr8s.get() today.

# kr8s/objects.py
from kr8s._objects import Pod as _Pod
from kr8s._async_utils import run_sync

class Pod(_Pod):
    """..."""

    ...

    @classmethod
    def list(cls, **kwargs) -> APIObject | list[APIObject]:
        """List objects in Kubernetes.

        Args:
            **kwargs: Keyword arguments to pass to :func:`kr8s.get`.

        Returns:
            A list of objects.
        """
        return run_sync(super().list)(**kwargs)

    ...

The upsides of this are:

• Static analysers like mypy would have an easier time parsing the code
• We could do away with the sphinx-autoapi plugin

But the downsides are:

• We would need to duplicate all methods, including inherited methods from the base class
• We would need to duplicate docstrings
• This would potentially add thousands of lines of boilerplate code
• We would need to find a way to keep everything in sync

[florianvazelle]

I've tried several approaches, but none seem to work for the first option.

The third option is also problematic because:

• MyPy doesn't allow overriding an async method with a sync one,
• The best return type would be list[Pod], but super().list actually returns an async APIObject.

There doesn't seem to be an easy fix

[jacobtomlinson]

Thanks for taking the time to dig into this.

I did think more about option 1 and I guess it's problematic because even if we write a plugin specifically for kr8s, all code that depends on kr8s would need the plugin, which is not something we can ask users to do.

I agree for option 3 it would be best to return list[Pod]. We could add a runtime assertion to make mypy happy and narrow the type. But as you say if you can't override async methods with sync ones then this option is not viable.

For option 2 I wonder if we need some custom stub generation script that we can invoke as part of the pre-commit hooks. This way we can explicitly tell mypy what to expect, but we don't need to worry about drift if they are auto-generated.