rerun-io / rerun

Visualize streams of multimodal data. Fast, easy to use, and simple to integrate. Built in Rust using egui.
https://rerun.io/
Apache License 2.0
6.31k stars 299 forks source link

Defaulting the RecordingId to `authkey()` should be opt-in #3793

Open nikolausWest opened 11 months ago

nikolausWest commented 11 months ago

UPDATE:


Describe the bug When I create two recordings with the same application id using the python SDK, only a single recording is shown in the viewer.

To Reproduce

import rerun as rr
rr.init("test", spawn=True)
rr.init("test", spawn=True)

Expected behavior The viewer should show two recordings with the application id "test"

Screenshots It only shows a single recording Screenshot 2023-10-11 at 10 30 40

Rerun version 0.9

nikolausWest commented 11 months ago

I put this on the 0.9.1 milestone but it's ok to move if it's too hard to get in in time.

teh-cmc commented 11 months ago

This works:

fn main() -> Result<(), Box<dyn std::error::Error>> {
    let rec =
        rerun::RecordingStreamBuilder::new("test").connect(rerun::default_server_addr(), None)?;
    let rec =
        rerun::RecordingStreamBuilder::new("test").connect(rerun::default_server_addr(), None)?;
    Ok(())
}

This works:

int main() {
    {
        auto rec = rr::RecordingStream("test");
        rec.connect("127.0.0.1:9876").throw_on_failure();
    }

    {
        auto rec = rr::RecordingStream("test");
        rec.connect("127.0.0.1:9876").throw_on_failure();
    }
}

This doesn't work:

rr.init("test")
rr.connect()
rr.init("test")
rr.connect()

This doesn't work:

rr.init("test", spawn=True)
rr.init("test", spawn=True)

This doesn't work:

rr.new_recording(application_id="test", make_default=True, spawn=True)
rr.new_recording(application_id="test", make_default=True, spawn=True)

This doesn't work:

rec = rr.new_recording(application_id="test", make_default=True)
rec.connect()
rec = rr.new_recording(application_id="test", make_default=True)
rec.connect()

This works:

rec = rr.new_recording(application_id="test1", make_default=True, spawn=True)
rec = rr.new_recording(application_id="test2", make_default=True, spawn=True)

And most importantly, this works:

rec = rr.new_recording(application_id="test", recording_id="a", make_default=True, spawn=True)
rec = rr.new_recording(application_id="test", recording_id="b", make_default=True, spawn=True)
teh-cmc commented 11 months ago

This actually behaves as expected, it's just surprising that the recording ID defaults to the process' authkey: this should be opt-in.

Decision:

emilk commented 11 months ago

I suggest we do this at the same time as we refactor init overall

jleibs commented 10 months ago