Skip to content

Commit eb14e16

Browse files
authored
Fix RecordingStream so it has a unique recording id when none is provided
Source-Ref: 126fbcc2ca36a96a7b037b20e972a0239d8e94f8
1 parent 0cfb10b commit eb14e16

File tree

3 files changed

+22
-2
lines changed

3 files changed

+22
-2
lines changed

rerun_py/rerun_sdk/rerun/__init__.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@
178178
from .recording_stream import (
179179
BinaryStream as BinaryStream,
180180
ChunkBatcherConfig as ChunkBatcherConfig,
181-
RecordingStream as RecordingStream,
181+
RecordingStream as RecordingStream, # noqa: TC001
182182
binary_stream as binary_stream,
183183
get_application_id as get_application_id,
184184
get_data_recording as get_data_recording,
@@ -350,13 +350,15 @@ def init(
350350
recording_id = str(recording_id)
351351

352352
if init_logging:
353-
RecordingStream(
353+
# Note: don't use `RecordingStream` here, because it overrides the default recording id behavior
354+
bindings.new_recording(
354355
application_id=application_id,
355356
recording_id=recording_id,
356357
make_default=True,
357358
make_thread_default=False,
358359
default_enabled=default_enabled,
359360
send_properties=send_properties,
361+
batcher_config=None,
360362
)
361363

362364
if spawn:

rerun_py/rerun_sdk/rerun/recording_stream.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,12 @@ def __init__(
310310

311311
if recording_id is not None:
312312
recording_id = str(recording_id)
313+
else:
314+
# We must absolutely avoid passing a None value to the bindings, because this will trigger the behavior
315+
# where it attempts to use the same recording id for parallel streams in the same process. When a
316+
# `RecordingStream` is explicitly created, the reuse of a recording id is not at all expected, though.
317+
# TODO(RR-1154): this is yet another consequence of stateless APIs not being first class
318+
recording_id = uuid.uuid4().hex
313319

314320
self.inner = bindings.new_recording(
315321
application_id=application_id,

rerun_py/tests/unit/test_multi_stream.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,18 @@ def test_init_twice() -> None:
2929
assert recording_id == rr.get_recording_id()
3030

3131

32+
def test_recording_stream_twice() -> None:
33+
"""For `RecordingStream`, the default should be to have new, unique recording ids."""
34+
35+
with rr.RecordingStream("rerun_example_bug") as rec:
36+
id1 = rec.get_recording_id()
37+
38+
with rr.RecordingStream("rerun_example_bug") as rec:
39+
id2 = rec.get_recording_id()
40+
41+
assert id1 != id2
42+
43+
3244
def test_isolated_streams(tmp_path: Path) -> None:
3345
rec1_path = f"{tmp_path}/rec1.rrd"
3446
rec2_path = f"{tmp_path}/rec2.rrd"

0 commit comments

Comments
 (0)