Skip to content

Commit

Permalink
Add scary warning that default recording_id = authkey (#3799)
Browse files Browse the repository at this point in the history
  • Loading branch information
teh-cmc authored and emilk committed Oct 12, 2023
1 parent f1c324f commit b02bb5e
Showing 1 changed file with 51 additions and 2 deletions.
53 changes: 51 additions & 2 deletions rerun_py/rerun_sdk/rerun/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import functools
import random
from typing import Any, Callable, TypeVar, cast
from uuid import UUID

import numpy as np

Expand Down Expand Up @@ -247,10 +248,11 @@ def _init_recording_stream() -> None:
_init_recording_stream()


# TODO(#3793): defaulting recording_id to authkey should be opt-in
def init(
application_id: str,
*,
recording_id: str | None = None,
recording_id: str | UUID | None = None,
spawn: bool = False,
init_logging: bool = True,
default_enabled: bool = True,
Expand All @@ -266,6 +268,26 @@ def init(
For more advanced use cases, e.g. multiple recordings setups, see [`rerun.new_recording`][].
!!! Warning
If you don't specify a `recording_id`, it will default to a random value that is generated once
at the start of the process.
That value will be kept around for the whole lifetime of the process, and even inherited by all
its subprocesses, if any.
This makes it trivial to log data to the same recording in a multiprocess setup, but it also means
that the following code will _not_ create two distinct recordings:
```
rr.init("my_app")
rr.init("my_app")
```
To create distinct recordings from the same process, specify distinct recording IDs:
```
from uuid import uuid4
rr.init("my_app", recording_id=uuid4())
rr.init("my_app", recording_id=uuid4())
```
Parameters
----------
application_id : str
Expand Down Expand Up @@ -322,6 +344,9 @@ def init(
# via `_register_on_fork` but it's worth being conservative.
cleanup_if_forked_child()

if recording_id is not None:
recording_id = str(recording_id)

if init_logging:
new_recording(
application_id=application_id,
Expand All @@ -348,10 +373,11 @@ def init(
_spawn()


# TODO(#3793): defaulting recording_id to authkey should be opt-in
def new_recording(
*,
application_id: str,
recording_id: str | None = None,
recording_id: str | UUID | None = None,
make_default: bool = False,
make_thread_default: bool = False,
spawn: bool = False,
Expand All @@ -362,6 +388,26 @@ def new_recording(
If you only need a single global recording, [`rerun.init`][] might be simpler.
!!! Warning
If you don't specify a `recording_id`, it will default to a random value that is generated once
at the start of the process.
That value will be kept around for the whole lifetime of the process, and even inherited by all
its subprocesses, if any.
This makes it trivial to log data to the same recording in a multiprocess setup, but it also means
that the following code will _not_ create two distinct recordings:
```
rr.init("my_app")
rr.init("my_app")
```
To create distinct recordings from the same process, specify distinct recording IDs:
```
from uuid import uuid4
rec = rr.new_recording(application_id="test", recording_id=uuid4())
rec = rr.new_recording(application_id="test", recording_id=uuid4())
```
Parameters
----------
application_id : str
Expand Down Expand Up @@ -432,6 +478,9 @@ def new_recording(
except Exception:
pass

if recording_id is not None:
recording_id = str(recording_id)

recording = RecordingStream(
bindings.new_recording(
application_id=application_id,
Expand Down

0 comments on commit b02bb5e

Please sign in to comment.