Skip to content

Introduce RecordingOpen viewer event#10654

Merged
Wumpf merged 6 commits into
mainfrom
jan/recording-open-event
Jul 15, 2025
Merged

Introduce RecordingOpen viewer event#10654
Wumpf merged 6 commits into
mainfrom
jan/recording-open-event

Conversation

@jprochazk
Copy link
Copy Markdown
Member

def event_handler(event):
  if event.type == "recording_open":
    print(recording_id)

viewer = Viewer()
viewer.on_event(event_handler)

@jprochazk jprochazk added 🕸️ web regarding running the viewer in a browser notebook Jupyter notebooks etc include in changelog labels Jul 15, 2025
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jul 15, 2025

Latest documentation preview deployed successfully.

Result Commit Link
593f2c7 https://landing-54twfc0je-rerun.vercel.app/docs

Note: This comment is updated whenever you push a commit.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jul 15, 2025

Web viewer built successfully. If applicable, you should also test it:

  • I have tested the web viewer
Result Commit Link Manifest
593f2c7 https://rerun.io/viewer/pr/10654 +nightly +main

Note: This comment is updated whenever you push a commit.

let entity_db = store_hub.entity_db_mut(store_id);
let is_new_store = matches!(&msg, LogMsg::SetStoreInfo(_msg));
if is_new_store && entity_db.store_kind() == StoreKind::Recording {
if msg_will_add_new_store && entity_db.store_kind() == StoreKind::Recording {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Before, this was firing off the event twice... Which is really strange. I didn't investigate too deeply, I changed it to check not only that it's a SetStoreInfo message, but also that the store actually doesn't exist yet. That fixed it for me. I assume the root cause is that we are sending multiple store info messages somewhere.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

huh, that's a bit worrisome 🤔
but your check makes sense regardless

@Wumpf Wumpf requested review from Wumpf and removed request for Wumpf July 15, 2025 18:55
Copy link
Copy Markdown
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.

seems low risk enough to add it to 0.24 last minute. Liking the doc strings very much, thanks!

@Wumpf Wumpf merged commit b1ed59f into main Jul 15, 2025
47 checks passed
@Wumpf Wumpf deleted the jan/recording-open-event branch July 15, 2025 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

include in changelog notebook Jupyter notebooks etc 🕸️ web regarding running the viewer in a browser

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants