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

Improved CLI: stdout streaming support #7094

Merged
merged 5 commits into from
Aug 8, 2024
Merged

Improved CLI: stdout streaming support #7094

merged 5 commits into from
Aug 8, 2024

Conversation

teh-cmc
Copy link
Member

@teh-cmc teh-cmc commented Aug 7, 2024

You can now do this:

cat docs/snippets/all/archetypes/*_rust.rrd | rerun rrd compact --max-rows 99999999 --max-bytes 999999999 | rerun rrd print

Also made sure that encoders and decoders accurately report the size of the data they plow through.

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!
  • If have noted any breaking changes to the log API in CHANGELOG.md and the migration guide

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

@teh-cmc teh-cmc added 🧑‍💻 dev experience developer experience (excluding CI) 🚀 performance Optimization, memory use, etc do-not-merge Do not merge this PR include in changelog CLI Related to the Rerun CLI labels Aug 7, 2024
@jleibs
Copy link
Member

jleibs commented Aug 7, 2024

The -o is gone everywhere

Disagree with this aspect. Being able to output to stdout is great, but I don't think it should be required.

Being explicit with -o is frequently more clear, esp in contexts where you aren't really sure about how your shell is going to handle pipes (thinking pixi, github runners, etc.)

crates/top/rerun/src/commands/rrd/merge_compact.rs Outdated Show resolved Hide resolved
@teh-cmc teh-cmc force-pushed the cmc/streaming_err_encoder branch from cb7fbda to 98dcaee Compare August 8, 2024 07:56
Base automatically changed from cmc/streaming_err_encoder to main August 8, 2024 07:56
@teh-cmc teh-cmc removed the do-not-merge Do not merge this PR label Aug 8, 2024
@teh-cmc
Copy link
Member Author

teh-cmc commented Aug 8, 2024

The -o is gone everywhere

Disagree with this aspect. Being able to output to stdout is great, but I don't think it should be required.

Being explicit with -o is frequently more clear, esp in contexts where you aren't really sure about how your shell is going to handle pipes (thinking pixi, github runners, etc.)

I agree with your disagreement 😄 I only removed that feature because it lead to very weird interaction with another feature that I didn't publish in the end.

I'll add those back.

@teh-cmc teh-cmc force-pushed the cmc/cli_imp_4_stdout branch from e7844f9 to 8d4e1ef Compare August 8, 2024 08:02
@teh-cmc teh-cmc force-pushed the cmc/cli_imp_4_stdout branch from f73f98e to 55e7b1a Compare August 8, 2024 08:18
@teh-cmc teh-cmc merged commit d5d070b into main Aug 8, 2024
28 of 29 checks passed
@teh-cmc teh-cmc deleted the cmc/cli_imp_4_stdout branch August 8, 2024 08:26
teh-cmc added a commit that referenced this pull request Aug 8, 2024
You can now do this:
```
cat docs/snippets/all/archetypes/*_rust.rrd | rerun rrd compact --max-rows 99999999 --max-bytes 999999999 | rerun rrd filter --timeline log_tick | rerun -
```

- Fixes #7048 
- DNM: requires #7094
@teh-cmc teh-cmc changed the title Improved CLI 4: stdout streaming support Improved CLI: stdout streaming support Aug 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLI Related to the Rerun CLI 🧑‍💻 dev experience developer experience (excluding CI) include in changelog 🚀 performance Optimization, memory use, etc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants