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

chore: enforce kwarg logging #7207

Merged
merged 17 commits into from
Feb 29, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Next Next commit
tests: fix broken tests
  • Loading branch information
wochinge committed Feb 28, 2024
commit c530238632fd25b20dabe4a0214ba275324552b6
6 changes: 3 additions & 3 deletions haystack/components/rankers/meta_field.py
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ def run(
# If all docs are missing self.meta_field return original documents
if len(docs_with_meta_field) == 0:
logger.warning(
"The parameter <meta_field> is currently set to {meta_field}', but none of the provided Documents with IDs {document_ids} have this meta key.\n"
"The parameter <meta_field> is currently set to '{meta_field}', but none of the provided Documents with IDs {document_ids} have this meta key.\n"
"Set <meta_field> to the name of a field that is present within the provided Documents.\n"
"Returning the <top_k> of the original Documents since there are no values to rank.",
meta_field=self.meta_field,
Expand All @@ -212,7 +212,7 @@ def run(

if len(docs_missing_meta_field) > 0:
logger.warning(
"The parameter <meta_field> is currently set to {meta_field}' but the Documents with IDs {document_ids} don't have this meta key.\n"
"The parameter <meta_field> is currently set to '{meta_field}' but the Documents with IDs {document_ids} don't have this meta key.\n"
"These Documents will be placed at the end of the sorting order.",
meta_field=self.meta_field,
document_ids=",".join([doc.id for doc in docs_missing_meta_field]),
Expand Down Expand Up @@ -254,7 +254,7 @@ def _parse_meta(
unique_meta_values = {doc.meta[self.meta_field] for doc in docs_with_meta_field}
if not all(isinstance(meta_value, str) for meta_value in unique_meta_values):
logger.warning(
"The parameter <meta_value_type> is currently set to {meta_field}', but not all of meta values in the "
"The parameter <meta_value_type> is currently set to '{meta_field}', but not all of meta values in the "
"provided Documents with IDs {document_ids} are strings.\n"
"Skipping parsing of the meta values.\n"
"Set all meta values found under the <meta_field> parameter to strings to use <meta_value_type>.",
Expand Down
126 changes: 113 additions & 13 deletions haystack/logging.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,31 +16,113 @@
class PatchedLogger(typing.Protocol):
"""Class which enables using type checkers to find wrong logger usage."""

def debug(self, msg: str, *, _: Any = None, **kwargs: Any) -> None:
def debug(
self,
msg: str,
*,
_: Any = None,
exc_info: Any = None,
stack_info: Any = False,
stacklevel: int = 1,
**kwargs: Any,
) -> None:
...

def info(self, msg: str, *, _: Any = None, **kwargs: Any) -> None:
def info(
self,
msg: str,
*,
_: Any = None,
exc_info: Any = None,
stack_info: Any = False,
stacklevel: int = 1,
**kwargs: Any,
) -> None:
...

def warn(self, msg: str, *, _: Any = None, **kwargs: Any) -> None:
def warn(
self,
msg: str,
*,
_: Any = None,
exc_info: Any = None,
stack_info: Any = False,
stacklevel: int = 1,
**kwargs: Any,
) -> None:
...

def warning(self, msg: str, *, _: Any = None, **kwargs: Any) -> None:
def warning(
self,
msg: str,
*,
_: Any = None,
exc_info: Any = None,
stack_info: Any = False,
stacklevel: int = 1,
**kwargs: Any,
) -> None:
...

def error(self, msg: str, *, _: Any = None, **kwargs: Any) -> None:
def error(
self,
msg: str,
*,
_: Any = None,
exc_info: Any = None,
stack_info: Any = False,
stacklevel: int = 1,
**kwargs: Any,
) -> None:
...

def critical(self, msg: str, *, _: Any = None, **kwargs: Any) -> None:
def critical(
self,
msg: str,
*,
_: Any = None,
exc_info: Any = None,
stack_info: Any = False,
stacklevel: int = 1,
**kwargs: Any,
) -> None:
...

def exception(self, msg: str, *, _: Any = None, **kwargs: Any) -> None:
def exception(
self,
msg: str,
*,
_: Any = None,
exc_info: Any = None,
stack_info: Any = False,
stacklevel: int = 1,
**kwargs: Any,
) -> None:
...

def fatal(self, msg: str, *, _: Any = None, **kwargs: Any) -> None:
def fatal(
self,
msg: str,
*,
_: Any = None,
exc_info: Any = None,
stack_info: Any = False,
stacklevel: int = 1,
**kwargs: Any,
) -> None:
...

def log(self, level: int, msg: str, *, _: Any = None, **kwargs: Any) -> None:
def log(
self,
level: int,
msg: str,
*,
_: Any = None,
exc_info: Any = None,
stack_info: Any = False,
stacklevel: int = 1,
**kwargs: Any,
) -> None:
...

def setLevel(self, level: int) -> None:
Expand All @@ -51,9 +133,13 @@ def patch_log_method_to_kwargs_only(func: typing.Callable) -> typing.Callable:
"""A decorator to make sure that a function is only called with keyword arguments."""

@functools.wraps(func)
def log_only_with_kwargs(msg, *, _: Any = None, **kwargs: Any) -> Any: # we need the `_` to avoid a syntax error
def log_only_with_kwargs(
msg, *, _: Any = None, exc_info: Any = None, stack_info: Any = False, stacklevel: int = 1, **kwargs: Any
) -> Any: # we need the `_` to avoid a syntax error
existing_extra = kwargs.pop("extra", {})
return func(msg, extra={**existing_extra, **kwargs})
return func(
msg, exc_info=exc_info, stack_info=stack_info, stacklevel=stacklevel, extra={**existing_extra, **kwargs}
)

return log_only_with_kwargs

Expand All @@ -63,10 +149,24 @@ def patch_log_with_level_method_to_kwargs_only(func: typing.Callable) -> typing.

@functools.wraps(func)
def log_only_with_kwargs(
level, msg, *, _: Any = None, **kwargs: Any # we need the `_` to avoid a syntax error
level,
msg,
*,
_: Any = None,
exc_info: Any = None,
stack_info: Any = False,
stacklevel: int = 1,
**kwargs: Any, # we need the `_` to avoid a syntax error
) -> Any:
existing_extra = kwargs.pop("extra", {})
return func(level, msg, extra={**existing_extra, **kwargs})
return func(
level,
msg,
exc_info=exc_info,
stack_info=stack_info,
stacklevel=stacklevel,
extra={**existing_extra, **kwargs},
)

return log_only_with_kwargs

Expand Down
2 changes: 1 addition & 1 deletion test/test_logging.py
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ class TestCompositeLogger:
("warning", "warning"),
("error", "error"),
("fatal", "critical"),
("exception", "exception"),
("exception", "error"),
("critical", "critical"),
],
)
Expand Down
Loading