Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

StructlogProcessor fails typechecking #146

Open
AWhetter opened this issue Oct 23, 2024 · 2 comments · May be fixed by #147
Open

StructlogProcessor fails typechecking #146

AWhetter opened this issue Oct 23, 2024 · 2 comments · May be fixed by #147

Comments

@AWhetter
Copy link

When typechecking the following code:

from structlog.typing import Processor

shared_processors: tuple[Processor, ...] = (
    structlog.contextvars.merge_contextvars,
    structlog.processors.add_log_level,
    structlog.processors.StackInfoRenderer(),
    structlog.dev.set_exc_info,
    structlog.processors.TimeStamper(fmt="iso", utc=True),
)

processors: list[Processor]
if sys.stderr.isatty():
    processors = [
        *shared_processors,
        structlog.dev.ConsoleRenderer(),
    ]
else:
    processors = [
        *shared_processors,
        structlog.processors.dict_tracebacks,
        ecs_logging.StructlogFormatter(),
    ]

structlog.configure(
    processors=processors,
    logger_factory=structlog.PrintLoggerFactory(),
    cache_logger_on_first_use=True,
)

mypy raises the following:

error: List item 2 has incompatible type "StructlogFormatter"; expected
"Callable[[Any, str, MutableMapping[str, Any]], Union[Mapping[str, Any], str, bytes, bytearray, tuple[Any, ...]]]"  [list-item]
            ecs_logging.StructlogFormatter(),
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
note: "StructlogFormatter.__call__" has type "Callable[[Arg(Any, '_'), Arg(str, 'name'), Arg(dict[str, Any], 'event_dict')], str]"

StructlogFormatter is annotated as accepting a dict[str, Any], but it needs to accept any MutableMapping[str, Any]. The code already conforms to this, so addressing this issue wil hopefully only involve updating the type annotation.

@xrmx
Copy link
Member

xrmx commented Oct 24, 2024

@AWhetter thanks for reporting. We should probably run mypy in CI too to avoid regressions. Any chance you can provide a pull request?

AWhetter added a commit to AWhetter/ecs-logging-python that referenced this issue Oct 24, 2024
@AWhetter
Copy link
Author

AWhetter commented Oct 24, 2024

I've made a pull request, and I'm in the process of getting the CLA signed by my employer.
Hopefully I'm running mypy in the way that you were intending. mypy is already run by the github actions, but I'm running this as part of the tests. It's probably not necessary to be testing that mypy operates correctly in all versions of Python, but it is and it works.

AWhetter added a commit to AWhetter/ecs-logging-python that referenced this issue Dec 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants