From 22f7402b0bf85fb5b7c46e585fb37c4a72c36050 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 23:43:55 +0000 Subject: [PATCH 1/2] Only require a time range for device stream and topic queries `download_data`, `iter_messages`, and `get_topics` no longer require `start` and `end` when querying by session, matching the API, which needs a time range only when querying by device. A range is still required with `device_id` or `device_name`. `get_topics(include_schemas=True)` now returns `None` for a topic's `schema` when the API omits it instead of raising. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_012yf4xszPJuZbmsVHpZeUGr --- foxglove/client/api.py | 25 +++++++++++++++++-------- tests/test_client.py | 30 ++++++++++++++++++++++++++++++ tests/test_data.py | 25 +++++++++++++++++++++++++ tests/test_episodes.py | 10 ++++------ 4 files changed, 76 insertions(+), 14 deletions(-) diff --git a/foxglove/client/api.py b/foxglove/client/api.py index ca5d7fd..79de069 100644 --- a/foxglove/client/api.py +++ b/foxglove/client/api.py @@ -577,7 +577,9 @@ def iter_messages( session_id: ID of a session to download data from session_key: Key of a session to download data from start: The earliest time from which to retrieve data. + Required with device_id or device_name. end: The latest time from which to retrieve data. + Required with device_id or device_name. topics: An optional list of topics to retrieve. All topics will be retrieved if this is omitted. decoder_factories: an optional list of :py:class:`~mcap.decoder.DecoderFactory` instances @@ -724,11 +726,9 @@ def _make_stream_link( raise RuntimeError( "recording_id or recording_key cannot be combined with a session" ) - needs_range = episode_id is None and (by_device or not by_recording) - if needs_range and (start is None or end is None): + if by_device and (start is None or end is None): raise RuntimeError( - "start and end must be provided unless using episode_id, " - "recording_id, or recording_key" + "start and end must be provided when using device_id or device_name" ) params = { @@ -790,7 +790,9 @@ def download_data( session_id: ID of a session to download data from session_key: Key of a session to download data from start: The earliest time from which to retrieve data. + Required with device_id or device_name. end: The latest time from which to retrieve data. + Required with device_id or device_name. topics: An optional list of topics to retrieve. All topics will be retrieved if this is omitted. output_format: The output format of the data, either .bag or .mcap, defaulting to .mcap. @@ -1795,8 +1797,11 @@ def get_topics( :param device_id: Optionally filter topics by this device ID. :param device_name: Optionally filter topics by this device name. :param start: Filter topics by this start time. + Required with ``device_id`` or ``device_name``. :param end: Filter topics by this end time. + Required with ``device_id`` or ``device_name``. :param include_schemas: Optionally include the schema in the response. + A topic's ``schema`` is ``None`` when the API returns none for it. :param project_id: Optional Project to filter topics by. :param session_id: ID of a session to list topics from :param session_key: Key of a session to list topics from @@ -1825,10 +1830,11 @@ def get_topics( or by_recording ): raise RuntimeError("episode_id cannot be combined with another identifier") - if episode_id is None and not by_recording and (start is None or end is None): + if (device_id is not None or device_name is not None) and ( + start is None or end is None + ): raise RuntimeError( - "start and end must be provided unless using episode_id, " - "recording_id, or recording_key" + "start and end must be provided when using device_id or device_name" ) response = self.__session.get( self.__url__("/v1/data/topics"), @@ -1864,7 +1870,10 @@ def get_topics( "schema_name": t["schemaName"], } if include_schemas: - result["schema"] = base64.b64decode(t["schema"]) + schema = t.get("schema") + result["schema"] = ( + base64.b64decode(schema) if schema is not None else None + ) results.append(result) return results diff --git a/tests/test_client.py b/tests/test_client.py index 3286f78..04e6905 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -1,3 +1,4 @@ +import base64 from datetime import datetime import pytest @@ -99,6 +100,35 @@ def test_get_topics_by_recording_without_time_range(): ) +@responses.activate +def test_get_topics_by_session_without_time_range_or_schema(): + responses.add( + responses.GET, + api_url("/v1/data/topics"), + match=[query_param_matcher({"sessionId": "ses_1", "includeSchemas": "true"})], + json=[ + { + "topic": "/a", + "version": "1", + "encoding": "json", + "schemaEncoding": "jsonschema", + "schemaName": "A", + "schema": base64.b64encode(b"{}").decode(), + }, + { + "topic": "/b", + "version": "1", + "encoding": "json", + "schemaEncoding": "jsonschema", + "schemaName": "B", + }, + ], + ) + topics = Client("test").get_topics(session_id="ses_1", include_schemas=True) + assert topics[0]["schema"] == b"{}" + assert topics[1]["schema"] is None + + def test_camelize(): assert camelize("a_field_name") == "aFieldName" assert camelize("aFieldName") == "aFieldName" diff --git a/tests/test_data.py b/tests/test_data.py index e476ae2..cfba10d 100644 --- a/tests/test_data.py +++ b/tests/test_data.py @@ -262,6 +262,31 @@ def test_download_data_by_recording_without_time_range(): assert Client("test").download_data(recording_id="rec_1") == b"mcap" +@responses.activate +def test_download_data_by_session_without_time_range(): + download_link = fake.url() + responses.add( + responses.POST, + api_url("/v1/data/stream"), + match=[ + json_params_matcher( + { + "sessionKey": "run-1", + "projectId": "prj_1", + "outputFormat": "mcap", + "topics": [], + }, + strict_match=True, + ) + ], + json={"link": download_link}, + ) + responses.add(responses.GET, download_link, body=b"mcap") + + data = Client("test").download_data(session_key="run-1", project_id="prj_1") + assert data == b"mcap" + + @responses.activate def test_streaming_upload(): upload_link = fake.url() diff --git a/tests/test_episodes.py b/tests/test_episodes.py index f724055..b9b4905 100644 --- a/tests/test_episodes.py +++ b/tests/test_episodes.py @@ -485,20 +485,18 @@ def test_download_without_episode_requires_time_range(): client.download_data(device_id="dev_1") assert str(raised.value) == ( - "start and end must be provided unless using episode_id, " - "recording_id, or recording_key" + "start and end must be provided when using device_id or device_name" ) -def test_get_topics_without_episode_requires_time_range(): +def test_get_topics_by_device_requires_time_range(): client = Client("test") with pytest.raises(RuntimeError) as raised: - client.get_topics(session_id="ses_1") + client.get_topics(device_name="robot", start=datetime(2025, 1, 1)) assert str(raised.value) == ( - "start and end must be provided unless using episode_id, " - "recording_id, or recording_key" + "start and end must be provided when using device_id or device_name" ) From 85e5b0086f7aef3e5edc11f0f9c14fd4860ac707 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 23:48:46 +0000 Subject: [PATCH 2/2] Test listing a project's topics without a time range Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_012yf4xszPJuZbmsVHpZeUGr --- tests/test_client.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/test_client.py b/tests/test_client.py index 04e6905..4eba99c 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -129,6 +129,18 @@ def test_get_topics_by_session_without_time_range_or_schema(): assert topics[1]["schema"] is None +@responses.activate +def test_get_topics_for_a_project_without_time_range(): + # The API needs a time range only when filtering by device. + responses.add( + responses.GET, + api_url("/v1/data/topics"), + match=[query_param_matcher({"projectId": "prj_1", "includeSchemas": "false"})], + json=[], + ) + assert Client("test").get_topics(project_id="prj_1") == [] + + def test_camelize(): assert camelize("a_field_name") == "aFieldName" assert camelize("aFieldName") == "aFieldName"