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

Switch base methods to return Self instead of BoundLoggerBase #659

Merged
merged 13 commits into from
Oct 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions src/structlog/_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,21 @@

from __future__ import annotations

import sys

from typing import Any, Iterable, Mapping, Sequence

from structlog.exceptions import DropEvent

from .typing import BindableLogger, Context, Processor, WrappedLogger


if sys.version_info >= (3, 11):
hynek marked this conversation as resolved.
Show resolved Hide resolved
from typing import Self
else:
from typing_extensions import Self


class BoundLoggerBase:
"""
Immutable context carrier.
Expand Down Expand Up @@ -62,7 +70,7 @@ def __eq__(self, other: object) -> bool:
def __ne__(self, other: object) -> bool:
return not self.__eq__(other)

def bind(self, **new_values: Any) -> BoundLoggerBase:
def bind(self, **new_values: Any) -> Self:
"""
Return a new logger with *new_values* added to the existing ones.
"""
Expand All @@ -72,7 +80,7 @@ def bind(self, **new_values: Any) -> BoundLoggerBase:
self._context.__class__(self._context, **new_values),
)

def unbind(self, *keys: str) -> BoundLoggerBase:
def unbind(self, *keys: str) -> Self:
"""
Return a new logger with *keys* removed from the context.

Expand All @@ -85,7 +93,7 @@ def unbind(self, *keys: str) -> BoundLoggerBase:

return bl

def try_unbind(self, *keys: str) -> BoundLoggerBase:
def try_unbind(self, *keys: str) -> Self:
"""
Like :meth:`unbind`, but best effort: missing keys are ignored.

Expand All @@ -97,7 +105,7 @@ def try_unbind(self, *keys: str) -> BoundLoggerBase:

return bl

def new(self, **new_values: Any) -> BoundLoggerBase:
def new(self, **new_values: Any) -> Self:
"""
Clear context and binds *new_values* using `bind`.

Expand Down
8 changes: 4 additions & 4 deletions src/structlog/stdlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ def bind(self, **new_values: Any) -> BoundLogger:
"""
Return a new logger with *new_values* added to the existing ones.
"""
return super().bind(**new_values) # type: ignore[return-value]
return super().bind(**new_values)

def unbind(self, *keys: str) -> BoundLogger:
"""
Expand All @@ -165,15 +165,15 @@ def unbind(self, *keys: str) -> BoundLogger:
Raises:
KeyError: If the key is not part of the context.
"""
return super().unbind(*keys) # type: ignore[return-value]
return super().unbind(*keys)

def try_unbind(self, *keys: str) -> BoundLogger:
"""
Like :meth:`unbind`, but best effort: missing keys are ignored.

.. versionadded:: 18.2.0
"""
return super().try_unbind(*keys) # type: ignore[return-value]
return super().try_unbind(*keys)

def new(self, **new_values: Any) -> BoundLogger:
"""
Expand All @@ -183,7 +183,7 @@ def new(self, **new_values: Any) -> BoundLogger:
those wrapped by `structlog.threadlocal.wrap_dict` when threads
are re-used.
"""
return super().new(**new_values) # type: ignore[return-value]
return super().new(**new_values)

def debug(self, event: str | None = None, *args: Any, **kw: Any) -> Any:
"""
Expand Down
8 changes: 8 additions & 0 deletions tests/typing/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,14 @@ async def typecheck_stdlib_async() -> None:
await logger.alog(logging.CRITICAL, "async log")


def typecheck_bound_logger_return() -> None:
blogger: structlog.BoundLogger = structlog.get_logger(__name__)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting find here, and perhaps the reason that this hasn't come up sooner:
This test would fail with structlog.BoundLogger before this PR, but it would pass with structlog.stdlib.BoundLogger.

Copy link
Contributor Author

@aThorp96 aThorp96 Oct 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mmm, after hitting this, I think this is actually the root of the issue I'm experiencing; I don't think I realized that in some places I was typing as a structlog.BoundLogger and other places I was typing as a structlog.stdlib.BoundLogger, and if I did, I did not expect there to be a difference. My assumption would have been that the former was a top-level export of the latter. At least then I think my type error woes are gone. I'll still get this PR over the finish line, since i think it's correct and valuable.

blog = blogger.bind(key1="value1", key2="value2", key3="value3")
blog = blog.unbind("key1")
blog = blog.try_unbind("bad_key")
blog = blog.new(new="value")


# Structured tracebacks and ExceptionRenderer with ExceptionDictTransformer
struct_tb: structlog.tracebacks.Trace = structlog.tracebacks.extract(
ValueError, ValueError("onoes"), None
Expand Down