Skip to content

feat(sandbox): add convience kwargs to stream output without manual iteration#28

Open
domphan-wandb wants to merge 4 commits intomainfrom
dom/print-streaming-output
Open

feat(sandbox): add convience kwargs to stream output without manual iteration#28
domphan-wandb wants to merge 4 commits intomainfrom
dom/print-streaming-output

Conversation

@domphan-wandb
Copy link
Collaborator

@domphan-wandb domphan-wandb commented Jan 15, 2026

Add print_output parameter to exec() for convenient output printing

Summary

Adds a print_output parameter to sandbox.exec() that controls whether output is printed in real-time. By default, exec() is silent (output available via result.stdout). For convenience during debugging, set print_output=True to auto-print output without manual iteration.

Motivation

There are use cases where streaming output is needed—watching long-running processes, debugging, or monitoring logs. Previously this required manual iteration:

process = sandbox.exec(["python", "script.py"])
for line in process.stdout:
    print(line, end="")
result = process.result()

This PR adds a convenience method to avoid that boilerplate:

result = sandbox.exec(["python", "script.py"], print_output=True).result()

This pattern will also be useful for a future CLI where aviato exec can leverage the same printing behavior.

API

  • print_output: bool = False — new parameter on exec()
  • AVIATO_EXEC_PRINT=1 — environment variable to enable printing globally
print_output Behavior
False (default) Silent — access output via result.stdout
True Auto-print — output printed in real-time

Design Decisions

Opt-in printing (silent by default)

We chose to make printing opt-in rather than opt-out because:

  • SDK users typically want programmatic control over output
  • Unexpected printing can pollute logs in automated pipelines
  • Explicit print_output=True clearly signals intent

No prefix on output

Currently, auto-printed output has no prefix (e.g., sandbox ID). This keeps the convenience option simple and unopinionated. If prefixed output is needed (e.g., for multi-sandbox scenarios), users can use manual iteration with their own formatting. We can revisit adding an optional prefix in the future if there's demand.

How to Review

  1. Implementation: Check src/aviato/_sandbox.py — the print_output parameter flows to _exec_streaming_async() where printing happens
  2. Environment variable logic: Verify AVIATO_EXEC_PRINT correctly overrides print_output=False
  3. Tests: Unit tests cover silent default, print_output=True, stderr handling, and env var behavior
  4. Run the example: python examples/streaming_exec.py demonstrates all three output patterns

Testing

# Unit tests
uv run pytest tests/unit/aviato/test_sandbox.py -v -k "print_output or silent or env_var"

# Integration tests
uv run pytest tests/integration/aviato/test_sandbox.py -v -k "print_output"

@iiilisan
Copy link
Collaborator

iiilisan commented Jan 15, 2026

Thanks for looking into this—this would be nice to have!

My one concern is that quiet=False inverts the mental model—you're disabling something to enable a feature. Compare with standard library patterns where True enables behavior:

subprocess.run(..., capture_output=True)
subprocess.run(..., text=True)
requests.get(..., stream=True)

Suggestion: Follow the subprocess convention with a stdout parameter:

import sys
result = sb.exec(["echo", "hi"], stdout=sys.stdout).result()  # tee to stdout
result = sb.exec(["echo", "hi"]).result()                     # capture only

This is familiar to Python developers, more flexible (any file-like object works), and removes the need for the env var workaround.

Another option could be a fluent .tee() method, which keeps the exec() signature clean:

result = sb.exec(["echo", "hi"]).tee().result()

@domphan-wandb
Copy link
Collaborator Author

domphan-wandb commented Jan 15, 2026

Thanks for looking into this—this would be nice to have!

My one concern is that quiet=False inverts the mental model—you're disabling something to enable a feature. Compare with standard library patterns where True enables behavior:

subprocess.run(..., capture_output=True)
subprocess.run(..., text=True)
requests.get(..., stream=True)

Suggestion: Follow the subprocess convention with a stdout parameter:

import sys
result = sb.exec(["echo", "hi"], stdout=sys.stdout).result()  # tee to stdout
result = sb.exec(["echo", "hi"]).result()                     # capture only

This is familiar to Python developers, more flexible (any file-like object works), and removes the need for the env var workaround.

Another option could be a fluent .tee() method, which keeps the exec() signature clean:

result = sb.exec(["echo", "hi"]).tee().result()

Thanks for the suggestion. I've changed the flag to be print_output to address the semantics of using a negative to enable something. I think viewing the output of the command is a common enough use case where having this convenience method makes sense, easily set with an environment variable.

Setting stdout and stderr to a file-like object goes a bit beyond flipping a switch. I definitely see it being useful, but I think we can address it in another PR to limit the scope of this one. Let me know what you think!

@NavarrePratt
Copy link
Collaborator

Dropping a 1-off comment before I get to a thorough review in case I don't get to it today:

I'm good with the print_output flag on exec as the design choice to enable this.

@iiilisan
Copy link
Collaborator

Note to self:

Not being able to redirect stdout/stderr separately seems a little limiting, though probably not a big deal in practice for a convenience flag aimed at interactive debugging. If we want to build a CLI that redirects output properly, we can still do that at that layer.

Overall, LGTM

iiilisan
iiilisan previously approved these changes Jan 16, 2026
## Streaming Output
## Output Handling

### Default: Silent
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: requires explicit handling 🤷

Copy link
Collaborator

@NavarrePratt NavarrePratt left a comment

Choose a reason for hiding this comment

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

Few small comments, but lgtm after cleaning them up.

| `cleanup_old_sandboxes.py` | Sync | `def main()` | Age-based cleanup |

The `exec()` method returns a `Process` object. Call `.result()` to block for the final `ProcessResult`. Iterate over `process.stdout` before calling `.result()` if you need real-time streaming output.
The `exec()` method returns a `Process` object. Call `.result()` to block for the final `ProcessResult`. By default, output is silent (access via `result.stdout`). For convenience, use `print_output=True` to print output in real-time without manual iteration.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why did we remove the note about how to handle streaming logging? We should keep that in here.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Same comment about the change in examples/README.md.

Comment on lines +79 to +81
Both stdout and stderr are printed to stdout. Set `AVIATO_EXEC_PRINT=1` to enable globally.


Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: extra blank line

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants