From e0d79853698a09ae29c4afbd08940661ca5cb797 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20=C4=8Cerm=C3=A1k?= Date: Wed, 9 Oct 2024 20:27:18 +0200 Subject: [PATCH] Fix number of lines returned with host logs' query argument (#5334) If no cursor is specified and negative num_skip is used, we're pointing one record back from the last one, so host logs always returned 101 lines as the default. This was also the case of the lines query argument that used the number directly as num_skip. Instead of doing that, point N-1 records to the back and then get N records. Handle 1 record and invalid numbers silently to avoid the need for error handling in unpractical edge cases. --- supervisor/api/host.py | 19 +++++++++++++++---- tests/api/__init__.py | 2 +- tests/api/test_host.py | 6 +++--- 3 files changed, 19 insertions(+), 8 deletions(-) diff --git a/supervisor/api/host.py b/supervisor/api/host.py index 50bfa9190..f6f99aac0 100644 --- a/supervisor/api/host.py +++ b/supervisor/api/host.py @@ -61,7 +61,7 @@ _LOGGER: logging.Logger = logging.getLogger(__name__) IDENTIFIER = "identifier" BOOTID = "bootid" -DEFAULT_RANGE = 100 +DEFAULT_LINES = 100 SCHEMA_OPTIONS = vol.Schema({vol.Optional(ATTR_HOSTNAME): str}) @@ -226,13 +226,24 @@ class APIHost(CoreSysAttributes): log_formatter = LogFormatter.VERBOSE if "lines" in request.query: - lines = request.query.get("lines", DEFAULT_RANGE) + lines = request.query.get("lines", DEFAULT_LINES) + try: + lines = int(lines) + except ValueError: + # If the user passed a non-integer value, just use the default instead of error. + lines = DEFAULT_LINES + finally: + # We can't use the entries= Range header syntax to refer to the last 1 line, + # and passing 1 to the calculation below would return the 1st line of the logs + # instead. Since this is really an edge case that doesn't matter much, we'll just + # return 2 lines at minimum. + lines = max(2, lines) # entries=cursor[[:num_skip]:num_entries] - range_header = f"entries=:-{lines}:" + range_header = f"entries=:-{lines-1}:{lines}" elif RANGE in request.headers: range_header = request.headers.get(RANGE) else: - range_header = f"entries=:-{DEFAULT_RANGE}:" + range_header = f"entries=:-{DEFAULT_LINES-1}:{DEFAULT_LINES}" async with self.sys_host.logs.journald_logs( params=params, range_header=range_header, accept=LogFormat.JOURNAL diff --git a/tests/api/__init__.py b/tests/api/__init__.py index 1b712021a..50b8d859a 100644 --- a/tests/api/__init__.py +++ b/tests/api/__init__.py @@ -6,7 +6,7 @@ from aiohttp.test_utils import TestClient from supervisor.host.const import LogFormat -DEFAULT_LOG_RANGE = "entries=:-100:" +DEFAULT_LOG_RANGE = "entries=:-99:100" async def common_test_api_advanced_logs( diff --git a/tests/api/test_host.py b/tests/api/test_host.py index 9c5d29016..b2312fae8 100644 --- a/tests/api/test_host.py +++ b/tests/api/test_host.py @@ -14,7 +14,7 @@ from supervisor.host.control import SystemControl from tests.dbus_service_mocks.base import DBusServiceMock from tests.dbus_service_mocks.systemd import Systemd as SystemdService -DEFAULT_RANGE = "entries=:-100:" +DEFAULT_RANGE = "entries=:-99:100" # pylint: disable=protected-access @@ -249,7 +249,7 @@ async def test_advaced_logs_query_parameters( await api_client.get("/host/logs?lines=53") journald_logs.assert_called_once_with( params={"SYSLOG_IDENTIFIER": coresys.host.logs.default_identifiers}, - range_header="entries=:-53:", + range_header="entries=:-52:53", accept=LogFormat.JOURNAL, ) @@ -277,7 +277,7 @@ async def test_advaced_logs_query_parameters( ) journald_logs.assert_called_once_with( params={"SYSLOG_IDENTIFIER": coresys.host.logs.default_identifiers}, - range_header="entries=:-53:", + range_header="entries=:-52:53", accept=LogFormat.JOURNAL, ) journal_logs_reader.assert_called_with(ANY, LogFormatter.VERBOSE)