canonical / tempo-k8s-operator

This charmed operator automates the operational procedures of running Grafana Tempo, an open-source tracing backend.
https://charmhub.io/tempo-k8s
Apache License 2.0
3 stars 3 forks source link

support typing for non-direct subclasses of charmbase in `@trace_charm` #145

Closed PietroPasotti closed 1 day ago

PietroPasotti commented 1 week ago
          Another argument for the approach is that, the `trace_charm` decorator has type annotation for `CharmBase` ([ref](https://github.com/canonical/tempo-k8s-operator/blob/fab9528391d4c2934dfb6359a06e28a0a4c01ece/lib/charms/tempo_k8s/v1/charm_tracing.py#L460))

I'm all for type annotations, but for charms that implement some other inheritance, this breaks LSP(pyright) goodies

Originally posted by @paulomach in https://github.com/canonical/tempo-k8s-operator/issues/128#issuecomment-2183619281

PietroPasotti commented 1 week ago

@paulomach can you show what typing error(s) you're seeing by the way? pyright seems to be fine with subclasses

image

Also decorating the superclass instead seems to be good.

paulomach commented 1 week ago

Hi @PietroPasotti , the following:

from typing import Type

class A:
    def foo(self):
        print("original foo")

def decorator():
    # def _decorator(cls):  <- untyped arg
    def _decorator(cls: Type[A]):
        setattr(cls, "_decorated", True)
        return cls

    return _decorator

@decorator()
class B(A):
    def foo(self):
        print("foo")

    def bar(self):
        print("bar")

if __name__ == "__main__":
    b = B()
    if b._decorated:
        print("b is decorated")
    b.foo()
    b.bar()  # here pyright tells "bar" is unknown. Removing Type on _decorator allows correct resolution
paulomach commented 1 week ago

@PietroPasotti this is fixed by PR#144 already, can close.