Skip to content

Conversation

@teh-cmc
Copy link
Member

@teh-cmc teh-cmc commented Mar 4, 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).


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 added 📖 documentation Improvements or additions to documentation do-not-merge Do not merge this PR include in changelog 🪵 Log & send APIs Affects the user-facing API for all languages labels Mar 4, 2024
@teh-cmc teh-cmc force-pushed the cmc/sdk_dataloader_8_the_end branch from 689cd22 to 152db29 Compare March 4, 2024 15:26
teh-cmc added a commit that referenced this pull request 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:
- [x] external loader ran manually (`python loader | rerun`)
- [x] external loader via rerun (`rerun xxx.py`)
- [x] log_file with external loader (`log_file xxx.py`)
- [x] external loader ran manually (`loader | rerun`)
- [x] external loader via rerun (`rerun xxx.rs`)
- [x] log_file with external loader (`log_file xxx.rs`)

---

Part of series of PR to expose configurable `DataLoader`s to our SDKs:
- #5327 
- #5328 
- #5330
- #5337
- #5351
- #5355
- #5379
- #5361
- #5388

---------

Co-authored-by: Andreas Reich <[email protected]>
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 teh-cmc force-pushed the cmc/sdk_dataloader_8_the_end branch from 152db29 to 27231a8 Compare March 4, 2024 16:53
@teh-cmc teh-cmc removed the do-not-merge Do not merge this PR label Mar 4, 2024
@teh-cmc teh-cmc force-pushed the cmc/sdk_dataloader_8_the_end branch 2 times, most recently from f8ba0c5 to c5edd3a Compare March 4, 2024 17:06
@teh-cmc teh-cmc force-pushed the cmc/sdk_dataloader_8_the_end branch from c5edd3a to 196c452 Compare March 4, 2024 17:13
@teh-cmc teh-cmc marked this pull request as ready for review March 4, 2024 17:14
@teh-cmc
Copy link
Member Author

teh-cmc commented Mar 4, 2024

We did it 🥳 cc @roym899

image

@Wumpf Wumpf self-requested a review March 5, 2024 09:06
Copy link
Member

@Wumpf Wumpf left a comment

Choose a reason for hiding this comment

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

neat! left suggestions to improve it a bit more still, but already a lot better

Copy link
Member

@Wumpf Wumpf left a comment

Choose a reason for hiding this comment

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

thanks for going the extra mile, appreciated!

@teh-cmc teh-cmc merged commit 8ebb5df into main Mar 6, 2024
@teh-cmc teh-cmc deleted the cmc/sdk_dataloader_8_the_end branch March 6, 2024 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

📖 documentation Improvements or additions to documentation include in changelog 🪵 Log & send APIs Affects the user-facing API for all languages

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DataLoaders should be exposed to the SDKs

3 participants