Skip to content

fix: add logs to devboxes, smoke tests & examples#755

Merged
james-rl merged 3 commits intomainfrom
james/sdk-logs
Mar 4, 2026
Merged

fix: add logs to devboxes, smoke tests & examples#755
james-rl merged 3 commits intomainfrom
james/sdk-logs

Conversation

@james-rl
Copy link
Contributor

@james-rl james-rl commented Mar 4, 2026

Adds missing oo-sdk devbox .logs method

  • Also added smoke tests & updated an example to show how you use it.

@james-rl james-rl requested a review from sid-rl March 4, 2026 17:44
@pytest.mark.timeout(THIRTY_SECOND_TIMEOUT)
def test_logs_basic(self, shared_devbox: Devbox) -> None:
"""Test retrieving devbox logs returns valid response structure."""
# Fetch logs - the API may return empty logs depending on timing
Copy link
Contributor

Choose a reason for hiding this comment

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

can we do a basic echo command to ensure that the logs aren't empty?

Comment on lines +1072 to +1073
execution = shared_devbox.cmd.exec_async('echo "filtered log test"')
result = execution.result()
Copy link
Contributor

Choose a reason for hiding this comment

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

why don't we just exec()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a great catch -- this is almost certainly caused by an issue with our agent guidance. I'm going to update this in a few places

logs = shared_devbox.logs(execution_id=execution.execution_id)

assert logs is not None
assert isinstance(logs.logs, list)
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need to add hasattr(logs, "logs") protection here as well? also can we check that the actual string we echo'd is in the logs?

logs = shared_devbox.logs(shell_name=shell_name)

assert logs is not None
assert isinstance(logs.logs, list)
Copy link
Contributor

Choose a reason for hiding this comment

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

same as above comment

Copy link
Contributor

@sid-rl sid-rl left a comment

Choose a reason for hiding this comment

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

LGTM!

@james-rl james-rl merged commit da5faa4 into main Mar 4, 2026
7 checks passed
@james-rl james-rl deleted the james/sdk-logs branch March 4, 2026 19:58
@stainless-app stainless-app bot mentioned this pull request Mar 4, 2026
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.

2 participants