Skip to content

Commit

Permalink
Fix number of lines returned with host logs' query argument (#5334)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
sairon authored Oct 9, 2024
1 parent 2968a57 commit e0d7985
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 8 deletions.
19 changes: 15 additions & 4 deletions supervisor/api/host.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@

IDENTIFIER = "identifier"
BOOTID = "bootid"
DEFAULT_RANGE = 100
DEFAULT_LINES = 100

SCHEMA_OPTIONS = vol.Schema({vol.Optional(ATTR_HOSTNAME): str})

Expand Down Expand Up @@ -226,13 +226,24 @@ async def advanced_logs_handler(
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
Expand Down
2 changes: 1 addition & 1 deletion tests/api/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

from supervisor.host.const import LogFormat

DEFAULT_LOG_RANGE = "entries=:-100:"
DEFAULT_LOG_RANGE = "entries=:-99:100"


async def common_test_api_advanced_logs(
Expand Down
6 changes: 3 additions & 3 deletions tests/api/test_host.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
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


Expand Down Expand Up @@ -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,
)

Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit e0d7985

Please sign in to comment.