Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix spawn starting the viewer even if logging is disabled #5284

Merged
merged 1 commit into from
Feb 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 19 additions & 3 deletions crates/re_sdk/src/recording_stream.rs
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,11 @@ impl RecordingStreamBuilder {
opts: &crate::SpawnOptions,
flush_timeout: Option<std::time::Duration>,
) -> RecordingStreamResult<RecordingStream> {
if !self.is_enabled() {
re_log::debug!("Rerun disabled - call to spawn() ignored");
return Ok(RecordingStream::disabled());
}

let connect_addr = opts.connect_addr();

// NOTE: If `_RERUN_TEST_FORCE_SAVE` is set, all recording streams will write to disk no matter
Expand Down Expand Up @@ -517,18 +522,19 @@ impl RecordingStreamBuilder {
/// This can be used to then construct a [`RecordingStream`] manually using
/// [`RecordingStream::new`].
pub fn into_args(self) -> (bool, StoreInfo, DataTableBatcherConfig) {
let enabled = self.is_enabled();

let Self {
application_id,
store_kind,
store_id,
store_source,
default_enabled,
enabled,
default_enabled: _,
enabled: _,
batcher_config,
is_official_example,
} = self;

let enabled = enabled.unwrap_or_else(|| crate::decide_logging_enabled(default_enabled));
let store_id = store_id.unwrap_or(StoreId::random(store_kind));
let store_source = store_source.unwrap_or_else(|| StoreSource::RustSdk {
rustc_version: env!("RE_BUILD_RUSTC_VERSION").into(),
Expand All @@ -555,6 +561,12 @@ impl RecordingStreamBuilder {

(enabled, store_info, batcher_config)
}

/// Internal check for whether or not logging is enabled using explicit/default settings & env var.
fn is_enabled(&self) -> bool {
self.enabled
.unwrap_or_else(|| crate::decide_logging_enabled(self.default_enabled))
}
}

// ----------------------------------------------------------------------------
Expand Down Expand Up @@ -1350,6 +1362,10 @@ impl RecordingStream {
opts: &crate::SpawnOptions,
flush_timeout: Option<std::time::Duration>,
) -> RecordingStreamResult<()> {
if !self.is_enabled() {
re_log::debug!("Rerun disabled - call to spawn() ignored");
return Ok(());
}
if forced_sink_path().is_some() {
re_log::debug!("Ignored setting new TcpSink since _RERUN_FORCE_SINK is set");
return Ok(());
Expand Down
4 changes: 4 additions & 0 deletions rerun_py/rerun_sdk/rerun/sinks.py
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,10 @@ def spawn(

"""

if not bindings.is_enabled():
logging.warning("Rerun is disabled - spawn() call ignored.")
return

import os
import subprocess
import sys
Expand Down
Loading