Skip to content

fix(python): add async SimpleBox metrics#960

Merged
DorianZheng merged 2 commits into
mainfrom
g4614/pol-101-simplebox-metrics
Jul 10, 2026
Merged

fix(python): add async SimpleBox metrics#960
DorianZheng merged 2 commits into
mainfrom
g4614/pol-101-simplebox-metrics

Conversation

@G4614

@G4614 G4614 commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Add metrics() to async SimpleBox so it matches the synchronous convenience API.

Test plan:

  • cargo fmt --all
  • sdks/python/.venv/bin/python -m py_compile sdks/python/boxlite/simplebox.py sdks/python/tests/test_simplebox.py
  • BOXLITE_DEPS_STUB=1 cargo check -p boxlite-python
  • Not rerun on debug host after splitting; the same SimpleBox metrics path passed on debug host before the split in fix(python): align SDK errors and metadata #954

Summary by CodeRabbit

  • New Features
    • Added an async metrics() capability to SimpleBox to retrieve resource usage (CPU/memory) for a box that has been started.
    • Enforces the same started/ready-state requirement as other SimpleBox actions.
  • Tests
    • Added an integration test that starts a SimpleBox, runs a command, retrieves box.metrics(), and verifies metrics are returned and include a non-zero commands_executed_total.

@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 3ae5ac5f-17e2-4f20-b238-066c838ef11a

📥 Commits

Reviewing files that changed from the base of the PR and between e58f075 and ec139c2.

📒 Files selected for processing (1)
  • sdks/python/tests/test_simplebox.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • sdks/python/tests/test_simplebox.py

📝 Walkthrough

Walkthrough

Adds an async metrics() method to SimpleBox that validates the box has been started before delegating to the underlying metrics call. Also adds an integration test that starts a box, runs a command, and asserts metrics are returned.

Changes

SimpleBox metrics support

Layer / File(s) Summary
metrics() method and integration test
sdks/python/boxlite/simplebox.py, sdks/python/tests/test_simplebox.py
SimpleBox.metrics() raises RuntimeError if not started, otherwise awaits self._box.metrics(); a new integration test starts a box, runs echo test, fetches metrics, and asserts a non-null result with commands_executed_total >= 1.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately names the main change: adding async SimpleBox metrics.
Description check ✅ Passed It covers the change summary and verification steps, though the template's Changes heading is not explicitly used.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch g4614/pol-101-simplebox-metrics

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@G4614
G4614 marked this pull request as ready for review July 9, 2026 13:47
@G4614
G4614 requested a review from a team as a code owner July 9, 2026 13:47
@boxlite-agent

boxlite-agent Bot commented Jul 9, 2026

Copy link
Copy Markdown

📦 BoxLite review — looks good · ec139c2

Review evidence

  • git diff --numstat origin/main...HEAD && git diff origin/main...HEAD — 2 files, +27/-0, small additive change
  • python3 -m py_compile boxlite/simplebox.py tests/test_simplebox.py — both files compile
  • pytest tests/test_simplebox.py (integration, needs runtime) — requires shared_runtime fixture/container env, not set up

Risk notes

  • API contract — new async metrics() mirrors sync_api/_simplebox.py metrics() and delegates to native self._box.metrics(); guarded by _started check like sibling methods (exec/stop)
  • native binding — self._box.metrics() implementation lives outside this repo's python source (rust binding), out of scope for diff review
  • tests — new integration test asserts commands_executed_total>=1 after exec; not executed here, only inspected
sdks/python/boxlite/simplebox.py
  SimpleBox.metrics  +9/-0  new async metrics wrapper
sdks/python/tests/test_simplebox.py
  test_simplebox_metrics  +18/-0  new integration test

reviewed ec139c2 in a BoxLite microVM · @boxlite-agent review to re-run · powered by BoxLite

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
sdks/python/tests/test_simplebox.py (1)

1-21: 📐 Maintainability & Code Quality | 🔴 Critical | ⚡ Quick win

Fix Python formatting failure.

The CI pipeline reports fmt:check:python would reformat this file. Run the formatter in write mode (e.g., make fmt:python or black --write) and commit the result before merging.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@sdks/python/tests/test_simplebox.py` around lines 1 - 21, Fix the Python
formatting issue in the async SimpleBox integration test by running the project
formatter in write mode and committing the resulting changes. The only affected
content is the test module containing test_simplebox_metrics and the async with
boxlite.SimpleBox block, so reformat that file to match the repository’s Python
style without changing test behavior.

Source: Pipeline failures

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@sdks/python/tests/test_simplebox.py`:
- Around line 1-21: Fix the Python formatting issue in the async SimpleBox
integration test by running the project formatter in write mode and committing
the resulting changes. The only affected content is the test module containing
test_simplebox_metrics and the async with boxlite.SimpleBox block, so reformat
that file to match the repository’s Python style without changing test behavior.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: c0b78cf6-72a6-4356-8cad-5010ff21f876

📥 Commits

Reviewing files that changed from the base of the PR and between 08de7ba and e58f075.

📒 Files selected for processing (2)
  • sdks/python/boxlite/simplebox.py
  • sdks/python/tests/test_simplebox.py

@DorianZheng
DorianZheng merged commit d029764 into main Jul 10, 2026
46 checks passed
@DorianZheng
DorianZheng deleted the g4614/pol-101-simplebox-metrics branch July 10, 2026 10:09
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