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

[HotFix] Fix Logging Service, Fix Posthog Handler Tests #6549

Merged
merged 1 commit into from
Jun 28, 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
11 changes: 6 additions & 5 deletions openbb_platform/core/openbb_core/app/logs/logging_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -185,8 +185,7 @@ def log(
],
custom_headers: Optional[Dict[str, Any]] = None,
):
"""
Log command output and relevant information.
"""Log command output and relevant information.

Parameters
----------
Expand All @@ -200,7 +199,10 @@ def log(
Callable representing the executed function.
kwargs : Dict[str, Any]
Keyword arguments passed to the function.
exec_info : Optional[Tuple[Type[BaseException], BaseException, Optional[TracebackType]]], optional
exec_info : Union[
Tuple[Type[BaseException], BaseException, TracebackType],
Tuple[None, None, None],
]
Exception information, by default None
"""
self._user_settings = user_settings
Expand All @@ -223,7 +225,7 @@ def log(
kwargs = {k: str(v)[:100] for k, v in kwargs.items()}

# Get execution info
error = str(exec_info[1]) if exec_info and len(exec_info) > 1 else None
error = None if all(i is None for i in exec_info) else str(exec_info[1])

# Construct message
message_label = "ERROR" if error else "CMD"
Expand All @@ -237,7 +239,6 @@ def log(
default=to_jsonable_python,
)
log_message = f"{message_label}: {log_message}"

log_level = logger.error if error else logger.info
log_level(
log_message,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,10 @@ def __init__(
system_settings: Optional[SystemSettings] = None,
):
"""Initialize the logging settings."""
user_settings = user_settings or UserSettings()
system_settings = system_settings or SystemSettings()

user_settings = user_settings if user_settings is not None else UserSettings()
system_settings = (
system_settings if system_settings is not None else SystemSettings()
)
user_data_directory = (
str(Path.home() / "OpenBBUserData")
if not user_settings.preferences
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,10 @@ def test_emit_calls_handleError_when_send_raises_exception(handler):
handler.handleError = MagicMock()

# Act
handler.emit(record)
try:
handler.emit(record)
except Exception as e:
assert isinstance(e, Exception)

# Assert
handler.send.assert_called_once_with(record=record)
Expand All @@ -132,7 +135,10 @@ def test_emit_calls_handleError_when_send_raises_exception_of_specific_type(hand
handler.handleError = MagicMock()

# Act
handler.emit(record)
try:
handler.emit(record)
except Exception as e:
assert isinstance(e, ValueError)

# Assert
handler.send.assert_called_once_with(record=record)
Expand All @@ -159,7 +165,10 @@ def test_emit_calls_handleError_when_send_raises_exception_of_another_type(handl
handler.handleError = MagicMock()

# Act
handler.emit(record)
try:
handler.emit(record)
except Exception as e:
assert isinstance(e, TypeError)

# Assert
handler.send.assert_called_once_with(record=record)
Expand Down
40 changes: 18 additions & 22 deletions openbb_platform/core/tests/app/logs/test_logging_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,34 +136,41 @@ class MockCredentials(BaseModel):


@pytest.mark.parametrize(
"user_settings, system_settings, route, func, kwargs, exec_info, custom_headers",
"user_settings, system_settings, route, func, kwargs, exec_info, custom_headers, expected_log_message",
[
(
"mock_settings",
"mock_system",
"mock_route",
"mock_func",
{},
(None, None, None),
None,
None,
'CMD: {"route": "mock_route", "input": {}, "error": null, "custom_headers": null}',
),
(
"mock_settings",
"mock_system",
"mock_route",
"mock_func",
{},
(OpenBBError, OpenBBError("mock_error")),
(
OpenBBError,
OpenBBError("mock_error"),
...,
), # ... is of TracebackType, but unnecessary for the test
{"X-OpenBB-Test": "test"},
'ERROR: {"route": "mock_route", "input": {}, "error": "mock_error", "custom_headers": {"X-OpenBB-Test": "test"}}', # noqa: E501
),
(
"mock_settings",
"mock_system",
"login",
"mock_func",
{},
None,
(None, None, None),
{"X-OpenBB-Test1": "test1", "X-OpenBB-Test2": "test2"},
"STARTUP",
),
],
)
Expand All @@ -176,6 +183,7 @@ def test_log(
kwargs,
exec_info,
custom_headers,
expected_log_message,
):
"""Test the log method."""
with patch(
Expand All @@ -198,9 +206,6 @@ def test_log(
mock_log_startup.assert_called_once()

else:
mock_info = mock_get_logger.return_value.info
mock_error = mock_get_logger.return_value.error

mock_callable = Mock()
mock_callable.__name__ = func

Expand All @@ -214,26 +219,17 @@ def test_log(
custom_headers=custom_headers,
)

message_label = "ERROR" if exec_info else "CMD"
log_message = json.dumps(
{
"route": route,
"input": kwargs,
"error": str(exec_info[1]) if exec_info else None,
"custom_headers": custom_headers,
}
)
log_message = f"{message_label}: {log_message}"

if exec_info:
if expected_log_message.startswith("ERROR"):
mock_error = mock_get_logger.return_value.error
mock_error.assert_called_once_with(
log_message,
expected_log_message,
extra={"func_name_override": "mock_func"},
exc_info=exec_info,
)
else:
if expected_log_message.startswith("CMD"):
mock_info = mock_get_logger.return_value.info
mock_info.assert_called_once_with(
log_message,
expected_log_message,
extra={"func_name_override": "mock_func"},
exc_info=exec_info,
)
Loading