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

Make sure all log messages are sent when using .serve() #6335

Merged
merged 14 commits into from
May 15, 2024
Merged

Conversation

emilk
Copy link
Member

@emilk emilk commented May 15, 2024

What

See commit messages for details

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!

To run all checks from main, comment on the PR with @rerun-bot full-check.

@emilk emilk added 🪳 bug Something isn't working 🕸️ web regarding running the viewer in a browser 🪵 Log & send APIs Affects the user-facing API for all languages include in changelog labels May 15, 2024
Copy link

github-actions bot commented May 15, 2024

Deployed docs

Commit Link
cd6bd79 https://landing-jl1bljws7-rerun.vercel.app/docs

@Wumpf Wumpf self-requested a review May 15, 2024 13:56
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.

lgtm!

crates/re_sdk/src/web_viewer.rs Show resolved Hide resolved
@emilk emilk merged commit 037697e into main May 15, 2024
29 of 34 checks passed
@emilk emilk deleted the emilk/serve-flush branch May 15, 2024 14:08
Wumpf pushed a commit that referenced this pull request 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`.
jleibs added a commit that referenced this pull request May 15, 2024
### What
This avoids a lint related to using blocking `recv()` on wasm buids.

This method was added as part of
#6335 but is not actually needed
by the viewer, only the SDK, which we don't build on wasm yet.

### 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/6339?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/6339?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/6339)
- [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`.

---------

Co-authored-by: Andreas Reich <[email protected]>
jleibs added a commit that referenced this pull request May 15, 2024
### What
This avoids a lint related to using blocking `recv()` on wasm buids.

This method was added as part of
#6335 but is not actually needed
by the viewer, only the SDK, which we don't build on wasm yet.

### 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/6339?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/6339?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/6339)
- [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`.

---------

Co-authored-by: Andreas Reich <[email protected]>
emilk pushed a commit that referenced this pull request May 20, 2024
### What

In #6335 `web_viewer` became a
default feature of `rerun_py`.
This caused a bunch of issues for development:
* `pixi run py-build` builds with default features and doesn't build new
wasm blobs, meaning that the build either fails or picks up outdated
* currently contributor ci doesn't build the web builder during its
wheel test job causing failures

_Q: Are we sure that wheel publish would still include the web_viewer?_
A: Yes. We test this prior to release. This works because we use the
`pypi` feature of `build_and_upload_wheels.py`. This script also ensures
that the wasm blobs are built.

_Q: Why does the non-contributor ci build the wasm blobs?_
A: It doesn't! Again, we use `build_and_upload_wheels.py` there which
builds with `--no-default-features`. However, that script can't be used
on the contributor ci since that would spill the GCS secrets (upload on
that script is not optional right now).


=> Alternative solutions:
* do `--no-default-features` on `pixi run py-build` and on the
contributor ci (potentially by using and adjusting
`build_and_upload_wheels.py`
* make `pixi run py-build` slower by making it depend on the web build 


### 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/6370?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/6370?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/6370)
- [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
🪳 bug Something isn't working include in changelog 🪵 Log & send APIs Affects the user-facing API for all languages 🕸️ web regarding running the viewer in a browser
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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