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

running with serve() of a really short script can make the web result close before data is rendering #6286

Closed
YoniChechik opened this issue May 10, 2024 · 3 comments · Fixed by #6335
Assignees
Labels
📖 documentation Improvements or additions to documentation 🪵 Log-API Affects the user-facing API for all languages
Milestone

Comments

@YoniChechik
Copy link

Describe the bug

running with serve() of a really short script can make the web result close before data is rendering, and getting the welcome screen instead.

To Reproduce
Steps to reproduce the behavior:

import time

import numpy as np
import rerun as rr

rr.init("data")

rr.serve()
# time.sleep(1) #ENABLE THIS AS A CURRENT WORKAROUND
positions = np.zeros((10, 3))
positions[:, 0] = np.linspace(-10, 10, 10)

colors = np.zeros((10, 3), dtype=np.uint8)
colors[:, 0] = np.linspace(0, 255, 10)

rr.log("my_points", rr.Points3D(positions, colors=colors, radii=0.5))

from this we get a welcome screen SOMETIMES instead of the points.

Expected behavior
Always get the results webview

Desktop (please complete the following information):

  • OS: windows 11, conda.

Rerun version

rerun_py 0.15.1 [rustc 1.74.0 (79e9716c9 2023-11-13), LLVM 17.0.4] x86_64-pc-windows-msvc release-0.15.1 7dedf88, built 2024-04-11T14:56:57Z

@YoniChechik YoniChechik added 👀 needs triage This issue needs to be triaged by the Rerun team 🪳 bug Something isn't working labels May 10, 2024
@Wumpf Wumpf added 🕸️ web regarding running the viewer in a browser and removed 👀 needs triage This issue needs to be triaged by the Rerun team labels May 13, 2024
@Wumpf Wumpf added this to the 0.16 milestone May 13, 2024
@emilk emilk added the 📖 documentation Improvements or additions to documentation label May 14, 2024
@emilk
Copy link
Member

emilk commented May 14, 2024

The problem here is that the server shuts down when the script shuts down.

We should improve the docs of serve() to explain this.

We should also consider other fixes:

  • Log a warning if the server shuts down to quickly
  • Add a block_on_exit argument or similar

Note that this problem affects all logging SDKs.

We could also consider always pausing for a second on shutdown if open_browser = True (default)

@emilk emilk added 🪵 Log-API Affects the user-facing API for all languages and removed 🪳 bug Something isn't working 🕸️ web regarding running the viewer in a browser labels May 14, 2024
@jleibs jleibs assigned jleibs and unassigned jleibs May 14, 2024
@emilk emilk self-assigned this May 15, 2024
@emilk
Copy link
Member

emilk commented May 15, 2024

I'm digging into this a little bit more.

If I run this simple script:

rr.init("rerun_example_data")
rr.serve()
rr.log("text", rr.TextDocument("Hello World!"))

My browser does start, downloads the viewer Wasm, and connects over web-socket - but the "Hello World!" string is never sent over!

@emilk
Copy link
Member

emilk commented May 15, 2024

The problem seems to be that WebViewerSink doesn't implement flush_blocking, so the last log message(s) may never arrive to the client.

emilk added a commit that referenced this issue May 15, 2024
### What
* Closes #6286

See commit messages for details

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested the web demo (if applicable):
* Using examples from latest `main` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6335?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6335?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG
* [x] If applicable, add a new check to the [release
checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)!

- [PR Build Summary](https://build.rerun.io/pr/6335)
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)

To run all checks from `main`, comment on the PR with `@rerun-bot
full-check`.
Wumpf pushed a commit that referenced this issue May 15, 2024
### What
* Closes #6286

See commit messages for details

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested the web demo (if applicable):
* Using examples from latest `main` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6335?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6335?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG
* [x] If applicable, add a new check to the [release
checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)!

- [PR Build Summary](https://build.rerun.io/pr/6335)
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)

To run all checks from `main`, comment on the PR with `@rerun-bot
full-check`.
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 🪵 Log-API Affects the user-facing API for all languages
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants