Spawn via $PATH 2: Redesign clap integration and clean up all examples#3997
Spawn via $PATH 2: Redesign clap integration and clean up all examples#3997
$PATH 2: Redesign clap integration and clean up all examples#3997Conversation
ec8de22 to
b9dd5c8
Compare
91f6edb to
72b1eca
Compare
b9dd5c8 to
c9f9336
Compare
Wumpf
left a comment
There was a problem hiding this comment.
this look veeeery nice and clean, also makes all the samples more lightweight by making them spawn instead of run.
But that return tuple bugs me a lot. Can we talk about alternatives later maybe?
| run(RecordingStream::disabled()); | ||
| return Ok(()); | ||
| } | ||
| pub fn init(&self, application_id: &str) -> anyhow::Result<(RecordingStream, ServeGuard)> { |
There was a problem hiding this comment.
The ergonomics of getting a tuple back where one part is super important and useful and the other an odd token I have to care around (but only when doing serve) are a bit odd. Even worse if I can init with serve from a scope now, it will not exit that scope iff I do serve (since the ServeGuard gets dropped). That's quite surprising behavior.
Suggestion for possible alternative: Given that the serve guard can't be killed, we might as well put it on a thread_local variable instead?
There was a problem hiding this comment.
I tried a lot of things, including putting the guard in a thread local, but the maybe-owned tokio runtime just makes things weird no matter what...
We do want the user to be able to drop the guard, it's the only to shutdown all the web stuff that we've spawned in the background :/
| let rec = rerun::RecordingStreamBuilder::new("rerun_example_dna_abacus") | ||
| .connect(rerun::default_server_addr(), rerun::default_flush_timeout())?; | ||
| .spawn(rerun::default_flush_timeout())?; |
There was a problem hiding this comment.
also need to fix this up in rust.md (the tutorial)
There was a problem hiding this comment.
They are so, so many places where we state the same things.. :(
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. https://github.com/rerun-io/rerun/assets/2910679/24eb5647-38c1-4049-b249-dc6e00e4ff54 --- Spawn via `$PATH` series: - #3996 - #3997 - #3998
c9f9336 to
db012ec
Compare
- 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
spawnfunctionality.RerunArgsto get rid of the awful callback while still dealing with Tokio's TLS shenanigans.The new
RerunArgscombined with the newspawnfrom PATH now make for a pretty nice experience:Spawn via
$PATHseries:$PATH1: Rust implementation #3996$PATH2: Redesignclapintegration and clean up all examples #3997$PATH3: C and C++ implementations #3998spawnin favor of fork-exec + binary artifacts #2109Checklist