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

SDK DataLoaders 7: stateful file logging #5379

Merged
merged 7 commits into from
Mar 4, 2024

Conversation

teh-cmc
Copy link
Member

@teh-cmc teh-cmc commented Mar 4, 2024

This makes the log_file APIs behave more like the standard log APIs; i.e. they inherit the state of their associated RecordingStream (app_id, rec_id, timepoint, etc...).

Also inherit the application ID while we're at it.

Makes the API much nicer to use and much more consistent with the rest.

Checks:

  • external loader ran manually (python loader | rerun)
  • external loader via rerun (rerun xxx.py)
  • log_file with external loader (log_file xxx.py)
  • external loader ran manually (loader | rerun)
  • external loader via rerun (rerun xxx.rs)
  • log_file with external loader (log_file xxx.rs)

Part of series of PR to expose configurable DataLoaders to our SDKs:

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 the web demo (if applicable):
  • The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG
  • If applicable, add a new check to the release checklist!

@teh-cmc teh-cmc force-pushed the cmc/sdk_dataloader_7_stateful_log_file branch from b932284 to 0811333 Compare March 4, 2024 11:07
@teh-cmc teh-cmc removed the do-not-merge Do not merge this PR label Mar 4, 2024
@teh-cmc teh-cmc marked this pull request as ready for review March 4, 2024 11:27
@Wumpf Wumpf self-requested a review March 4, 2024 14:31
@teh-cmc teh-cmc mentioned this pull request Mar 4, 2024
5 tasks
crates/re_data_source/src/data_loader/mod.rs Outdated Show resolved Hide resolved
Comment on lines 1098 to +1099
filepath: impl AsRef<std::path::Path>,
entity_path_prefix: Option<EntityPath>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not first put the entity path like with all other logging functions 🤔

edit: right, because that messes up languages with default arguments

Co-authored-by: Andreas Reich <andreas@rerun.io>
@teh-cmc teh-cmc merged commit 7b314d8 into main Mar 4, 2024
26 checks passed
@teh-cmc teh-cmc deleted the cmc/sdk_dataloader_7_stateful_log_file branch March 4, 2024 16:45
teh-cmc added a commit that referenced this pull request Mar 4, 2024
Introduces the new `DataLoaderSettings` business to C++ and update
examples accordingly (`external_data_loader` & `log_file`).

```bash
./build/debug/examples/cpp/log_file/example_log_file --recording-id this-one --entity-path-prefix a/b/c  --time sim_time=1000 --time wall_time=1709204046 --sequence sim_frame=42 rerun_cpp/tests/main.cpp | rerun -
```

![image](https://github.com/rerun-io/rerun/assets/2910679/b979a24c-29b6-473b-91b1-de3832bea436)


Checks:
- [x] external loader ran manually (`loader.exe | rerun`)
- [x] external loader via rerun (`rerun xxx.cpp`)
- [x] log_file with external loader (`log_file xxx.cpp`)

---

Part of series of PR to expose configurable `DataLoader`s to our SDKs:
- #5327 
- #5328 
- #5330
- #5337
- #5351
- #5355
- #5379
- #5361
- #5388
teh-cmc added a commit that referenced this pull request Mar 6, 2024
I guess that's good enough 🤷. I don't know, my brain has been
completely friend by C++ non-sense all day.

This includes a fix to make sure that a viewer that was spawned from the
python SDK is still allowed to spawn dataloaders implemented in python
(`RERUN_APP_ONLY` shenaniganeries).

- Fixes #4526 

---

Part of series of PR to expose configurable `DataLoader`s to our SDKs:
- #5327 
- #5328 
- #5330
- #5337
- #5351
- #5355
- #5379
- #5361
- #5388
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
include in changelog 🪵 Log & send APIs Affects the user-facing API for all languages 🐍 Python API Python logging API 🦀 Rust API Rust logging API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants