Spawn via `$PATH` 1: Rust implementation #3996

teh-cmc · 2023-10-25T08:45:32Z

Introduces standalone rerun::spawn(SpawnOptions) API that allows one to start a Rerun Viewer process ready to listen for TCP connections, as well as the associated RecordingStream integration.

23-10-25_11.40.45.patched.mp4

Spawn via $PATH series:

Checklist

I have read and agree to Contributor Guide and the Code of Conduct
I've included a screenshot or gif (if applicable)
I have tested demo.rerun.io (if applicable)
The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG

emilk

Lovely to finally get the ball rolling on this!

crates/re_sdk/src/recording_stream.rs

crates/re_sdk/src/spawn.rs

docs/code-examples/bar_chart.rs

crates/re_sdk/src/spawn.rs

emilk · 2023-10-26T07:42:28Z

crates/re_sdk/src/spawn.rs

+    if TcpStream::connect_timeout(&connect_addr, Duration::from_millis(1)).is_ok() {
+        re_log::info!(
+            addr = %opts.listen_addr(),
+            "A process is already listening at this address. Assuming it's a Rerun Viewer."
+        );
+        return Ok(());
+    }


To return Ok when perhaps there is a completely unrelated (non-rerun) process already occupying the port seems like trouble waiting to happen…

To me it makes sense it it is a special error (SpawnError::AddressAlreadyInUse) and have the caller do the assuming instead, but it's not a blocker. At least the logging is loud and clear!

We've used the same trick in the Python SDK for months without issue.
I'll make an issue to introduce a proper Rerun handshake, but this is out of the scope of this PR.

emilk · 2023-10-26T07:52:52Z

crates/re_types/src/archetypes/arrows3d.rs

@@ -30,7 +30,7 @@ use ::re_types_core::{DeserializationError, DeserializationResult};
 /// use std::f32::consts::TAU;
 ///
 /// fn main() -> Result<(), Box<dyn std::error::Error>> {
-///     let (rec, storage) = rerun::RecordingStreamBuilder::new("rerun_example_arrow3d").memory()?;


All of our examples now pass None as the flush_timeout. Reading the docs:

flush_timeout is the minimum time the [TcpSink][crate::log_sink::TcpSink] will
wait during a flush before potentially dropping data. Note: Passing None here can cause a
call to flush to block indefinitely if a connection cannot be established.

At the same time we have re_sdk::default_flush_timeout() which return Some(2s)

So we are effectively teaching users (via example) not to use the default. I think this is weird.

At the same time, the examples in the docs for spawn and spawn_opts DOES make use of this, so even our examples are inconsistent.

What do we want our examples to behave? What do we want our users to do?

Without having thought through the actual flushing consequences, I would suggest that spawn() takes no arguments so that we pick the same default for all our examples (and for our users!).

spawn is effectively just a wrapper around connect so I want them to be consistent, and users definitely need to be able to customize this...

We teach users to use re_sdk::default_flush_timeout() anytime we mention connect(), I'll do the same for spawn() then.

…or we use the same pattern of connect() and connect_opts(…).

I really wish Rust has default arguments 😭

…or we use the same pattern of connect() and connect_opts(…).

That could be quite good. I'll give it a shot in a follow up.

I really wish Rust has default arguments 😭

:yes:

…mples (#3997) - Get rid of the old thread-based `spawn` functionality. - Redesign `RerunArgs` to get rid of the awful callback while still dealing with Tokio's TLS shenanigans. - Update all tests and examples. The new `RerunArgs` combined with the new `spawn` from PATH now make for a pretty nice experience: ```rust let args = Args::parse(); let (rec, _serve_guard) = args.rerun.init("my_app")?; // do stuff with rec ``` --- Spawn via `$PATH` series: - #3996 - #3997 - #3998 --- - Fixes #2109

- Introduces standalone `rr_spawn(rr_spawn_options)` C API that allows one to start a Rerun Viewer process ready to listen for TCP connections, as well as the associated `rr_recording_stream` integration. - Introduces standalone `rerun::spawn()` C++ API that allows one to start a Rerun Viewer process ready to listen for TCP connections, as well as the associated `RecordingStream` integration. https://github.com/rerun-io/rerun/assets/2910679/8ae5003a-78b2-4e75-91d3-6dc4b8dd22ac --- Spawn via `$PATH` series: - #3996 - #3997 - #3998 --- - Fixes #3757 - Fixes #3942

Last one. Spawn via `$PATH` series: - #3996 - #3997 - #3998 - #4013

teh-cmc added 5 commits October 25, 2023 10:03

introduce re_sdk::spawn(SpawnOptions)

b9384a2

introduce RecordingStream::spawn

aa2c6a5

update API examples to use spawn instead

aff7d68

update rust API example template

46bd0e7

add spawn_viewer example

73254e8

teh-cmc added 🦀 Rust API Rust logging API include in changelog labels Oct 25, 2023

teh-cmc changed the title ~~Spawn via $PATH 1: Rust implementation~~ Spawn via $PATH 1: Rust implementation Oct 25, 2023

linkinator pls behave

6d0ad42

This was referenced Oct 25, 2023

Spawn via $PATH 2: Redesign clap integration and clean up all examples #3997

Merged

Spawn via $PATH 3: C and C++ implementations #3998

Merged

fix log_integration example race

f6b9893

teh-cmc marked this pull request as ready for review October 25, 2023 09:43

teh-cmc added the 🧑‍💻 dev experience developer experience (excluding CI) label Oct 25, 2023

emilk self-requested a review October 25, 2023 11:12

emilk requested changes Oct 25, 2023

View reviewed changes

teh-cmc force-pushed the cmc/binary_spawn_1_rust branch from a966f54 to 91f6edb Compare October 25, 2023 14:52

address PR comments

72b1eca

teh-cmc force-pushed the cmc/binary_spawn_1_rust branch from 91f6edb to 72b1eca Compare October 25, 2023 14:58

teh-cmc requested a review from emilk October 25, 2023 17:30

teh-cmc mentioned this pull request Oct 25, 2023

Spawn via $PATH 4: Updated docs & misc clean up #4013

Merged

4 tasks

emilk reviewed Oct 26, 2023

View reviewed changes

address PR comments

eff9ed5

teh-cmc force-pushed the cmc/binary_spawn_1_rust branch from a6c81dc to eff9ed5 Compare October 26, 2023 10:39

teh-cmc requested a review from emilk October 26, 2023 10:44

emilk approved these changes Oct 26, 2023

View reviewed changes

fmt

33cd729

teh-cmc merged commit c7d26a1 into main Oct 26, 2023
22 of 27 checks passed

teh-cmc deleted the cmc/binary_spawn_1_rust branch October 26, 2023 11:25

teh-cmc added a commit that referenced this pull request Oct 26, 2023

Spawn via $PATH 4: Updated docs & misc clean up (#4013)

55d25ac

Last one. Spawn via `$PATH` series: - #3996 - #3997 - #3998 - #4013

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Spawn via `$PATH` 1: Rust implementation #3996

Spawn via `$PATH` 1: Rust implementation #3996

teh-cmc commented Oct 25, 2023 •

edited by github-actions bot

emilk left a comment

emilk Oct 26, 2023

teh-cmc Oct 26, 2023

emilk Oct 26, 2023

teh-cmc Oct 26, 2023

emilk Oct 26, 2023

teh-cmc Oct 26, 2023

Spawn via $PATH 1: Rust implementation #3996

Spawn via $PATH 1: Rust implementation #3996

Conversation

teh-cmc commented Oct 25, 2023 • edited by github-actions bot

Checklist

emilk left a comment

Choose a reason for hiding this comment

emilk Oct 26, 2023

Choose a reason for hiding this comment

teh-cmc Oct 26, 2023

Choose a reason for hiding this comment

emilk Oct 26, 2023

Choose a reason for hiding this comment

teh-cmc Oct 26, 2023

Choose a reason for hiding this comment

emilk Oct 26, 2023

Choose a reason for hiding this comment

teh-cmc Oct 26, 2023

Choose a reason for hiding this comment

Spawn via `$PATH` 1: Rust implementation #3996

Spawn via `$PATH` 1: Rust implementation #3996

teh-cmc commented Oct 25, 2023 •

edited by github-actions bot