Skip to content

(CubeAPI)examples,docs: add snapshot/rollback/clone demo suite and guide#374

Merged
ls-ggg merged 1 commit into
TencentCloud:masterfrom
ls-ggg:feat/snapshot-rollback-clone-examples
May 27, 2026
Merged

(CubeAPI)examples,docs: add snapshot/rollback/clone demo suite and guide#374
ls-ggg merged 1 commit into
TencentCloud:masterfrom
ls-ggg:feat/snapshot-rollback-clone-examples

Conversation

@ls-ggg
Copy link
Copy Markdown
Collaborator

@ls-ggg ls-ggg commented May 27, 2026

  • Add CubeAPI/examples/snapshot-rollback-clone/ with 11 runnable demos covering create, list, clone, state preservation, snapshot lifecycle, concurrent clone, fork, rollback, and delete workflows
  • Add docs/guide/snapshot-rollback-clone.md (English) and docs/zh/guide/snapshot-rollback-clone.md (Chinese) with concept diagrams and minimal code snippets

@cubesandboxbot
Copy link
Copy Markdown

cubesandboxbot Bot commented May 27, 2026

Claude finished @ls-ggg's task in 1m 38s —— View job


Review: PR #374 — Snapshot/Rollback/Clone Demo Suite & Guide

I've reviewed all 17 files (+897/-0 lines). Overall the code is clean, well-structured, and the documentation is thorough. I found a few issues worth addressing before merging.


🐛 Bugs (should fix)

1. Broken link in CubeAPI/examples/snapshot-rollback-clone/README.md

Line 73 links to ../../../docs/en/guide/snapshot-rollback-clone.md, but the English guide is at docs/guide/snapshot-rollback-clone.md (no /en/ segment). This is a 404 after merge.

- [Guide: Snapshot, Rollback & Clone](../../../docs/en/guide/snapshot-rollback-clone.md)

→ should be:

- [Guide: Snapshot, Rollback & Clone](../../../docs/guide/snapshot-rollback-clone.md)

The Chinese README_zh.md correctly links to ../../../docs/zh/guide/snapshot-rollback-clone.md — this is fine.


2. Version mismatch: requirements.txt vs READMEs and SDK

File Version constraint
requirements.txt cubesandbox>=0.3.0
README.md >=0.2.0
README_zh.md >=0.2.0
docs/guide/snapshot-rollback-clone.md >=0.2.0
docs/zh/guide/snapshot-rollback-clone.md >=0.2.0

The SDK is currently at 0.2.0 (per commit 31162ae). No 0.3.0 release exists yet. The requirements.txt should match the READMEs at >=0.2.0, or if 0.3.0 is intended to gate specific APIs used by these demos (e.g., clone(), rollback()), it should be bumped only after the SDK is actually released at 0.3.0.


💡 Suggestions (nice to fix)

3. env.py — Unused variable API_URL

API_URL is assigned at env.py:16 but never imported by any demo script. The SDK reads CUBE_API_URL from os.environ directly (confirmed in sdk/python/cubesandbox/_config.py). Consider either:

  • Importing it in each script and passing to the SDK to make it explicit, or
  • Removing the assignment and keeping only the docstring comment about expected env vars (since SSL_CERT_FILE is also documented but not read by env.py)

Not a blocker, but as-is it could confuse readers who see API_URL assigned but never consumed.

4. 09_rollback.py — Missing defensive check on r.logs.stdout

04_state_preserved.py, 06_clone_n.py, 07_clone_concurrent.py, and 08_fork_three_axis.py all use the pattern:

marker = r.logs.stdout[0].strip() if r.logs.stdout else ""

But 09_rollback.py:36,45 and 10_rollback_then_continue.py:23,29,38 access r.logs.stdout[0].strip() without the defensive guard. If the executed code produces no stdout (unlikely here, but possible under error conditions), this raises an IndexError. Consider making these consistent with the other demos.

5. env.pyrequirements.txt constraint should reference env.py dependency

The env.py module directly imports from cubesandbox import Sandbox, so it implicitly depends on cubesandbox. If requirements.txt is the canonical dependency file, this is fine. No action needed, just an observation.


✅ What looks good

  • Consistent structure: All 11 demos follow the same header format (copyright + docstring) and cleanup pattern.
  • Self-contained: Each script is independently runnable and clearly labeled.
  • Good documentation comments: The docstrings explain both the "what" and the "why" (e.g., explaining that clone() internally creates/deletes a snapshot transparently).
  • Clean pagination: The list_snapshots() pagination logic in demos 02, 05, and 11 is correct and consistent.
  • Thorough test coverage: The demos cover the full surface area — create, list, clone, fork (inheritance + isolation + continuity), rollback, rollback-then-continue, and delete.
  • README table of contents: Excellent reference table linking demo number to script name and API topic.
  • Guide documentation: Clear mermaid diagram, well-organized sections, and the best-practices section is a nice addition.

Summary

Severity Count Action
Bug 2 Fix broken README link and version constraint
Suggestion 2 Consider defensive stdout checks and env.py cleanup

The two bugs should be fixed before merging; the suggestions are optional polish.

@ls-ggg ls-ggg force-pushed the feat/snapshot-rollback-clone-examples branch from 6f5f5d4 to 011abfe Compare May 27, 2026 08:18
@cubesandboxbot
Copy link
Copy Markdown

cubesandboxbot Bot commented May 27, 2026

Claude finished @ls-ggg's task in 1m 38s —— View job


Review — PR #374: Snapshot/Rollback/Clone Demo Suite & Guide

Reviewing all 17 files...


Previous review findings — rechecked

The prior review flagged 4 issues. After reading every file in the current diff, none of those issues exist in the code:

  1. Broken README linkREADME.md:73 already links to ../../../docs/guide/snapshot-rollback-clone.md (correct, no /en/ segment). The Chinese README_zh.md:73 links to ../../../docs/zh/guide/snapshot-rollback-clone.md (also correct).

  2. Version mismatchrequirements.txt says cubesandbox>=0.2.0, consistent with both READMEs (>=0.2.0) and both docs/guides (>=0.2.0). No mismatch.

  3. Unused API_URL in env.pyenv.py only exports TEMPLATE_ID. No API_URL variable exists in the file.

  4. Missing defensive stdout checks — All demo files that access r.logs.stdout[0] use the guard r.logs.stdout[0].strip() if r.logs.stdout else "". This includes 09_rollback.py:37,47 and 10_rollback_then_continue.py:24,31,41.

The code was already correct — this review proceeds from the actual diff state.


🔍 New findings

1. Documentation listing example — first-page items are silently dropped

Both docs/guide/snapshot-rollback-clone.md (lines 69–71) and docs/zh/guide/snapshot-rollback-clone.md (lines 69–71) show this pagination pattern:

items, token = Sandbox.list_snapshots()
while token:
    items, token = Sandbox.list_snapshots(next_token=token)

This fetches the first page, discards items, then loops only over pages 2+. The actual demo scripts (02_list_snapshots.py, 05_snapshot_outlives_sandbox.py, 11_delete_snapshot.py) correctly use a while True / if not token: break pattern that processes every page. The docs should match the demos:

items, token = Sandbox.list_snapshots()
while True:
    for snap in items:
        print(f"  {snap.snapshot_id}")
    if not token:
        break
    items, token = Sandbox.list_snapshots(next_token=token)

This is consistent except that there's one issue. In line 69 of the docs:

items, token = Sandbox.list_snapshots()

This actually does process the first page... No wait, looking more carefully at the docs snippet:

# List all snapshots
items, token = Sandbox.list_snapshots()
while token:
    items, token = Sandbox.list_snapshots(next_token=token)

The items from the first call is never used — the loop immediately enters on token and overwrites items. The variable items after the loop holds only the last page, not the union of all pages. Since these are illustrative snippets in documentation rather than runnable demos, the intent is to show the pagination API, but the pattern is missing the inner processing loop.

Fix this →


✅ What looks good

  • Consistent structure: All 11 demos follow the same header format (copyright + docstring) and cleanup pattern.
  • Self-contained: Each script is independently runnable and clearly labeled in its docstring.
  • Thorough coverage: The demos cover the full surface area — create, list, clone, state preservation, snapshot lifecycle, clone-n, concurrent clone, fork semantics (inheritance + isolation + continuity), rollback, rollback-then-continue, and delete.
  • Solid defensive patterns: The stdout[0].strip() if stdout else "" guard is used correctly everywhere.
  • Clean pagination: The while True / if not token: break pattern in the demos is correct and consistent.
  • Guide documentation: Clear mermaid diagram, well-organized sections, and the best-practices section with practical advice.

Summary

Severity Count Action
Bug 1 Fix the listing example in both docs to not drop first-page items
Suggestion 0 Clean code overall

One real issue found (in documentation snippets, not in the executable demos). The demos themselves are well-written. Happy to implement the doc fix if desired.

- Add CubeAPI/examples/snapshot-rollback-clone/ with 11 runnable demos
  covering create, list, clone, state preservation, snapshot lifecycle,
  concurrent clone, fork, rollback, and delete workflows
- Add docs/guide/snapshot-rollback-clone.md (English) and
  docs/zh/guide/snapshot-rollback-clone.md (Chinese) with concept
  diagrams and minimal code snippets

Signed-off-by: ls-ggg <335814617@qq.com>
@ls-ggg ls-ggg force-pushed the feat/snapshot-rollback-clone-examples branch from 011abfe to 1d07886 Compare May 27, 2026 08:23
@cubesandboxbot
Copy link
Copy Markdown

cubesandboxbot Bot commented May 27, 2026

Claude finished @ls-ggg's task in 1m 33s —— View job


Review: PR #374 — Snapshot/Rollback/Clone Demo Suite & Guide

Reviewing all 17 files (+897/-0) from scratch...


🔍 Summary

After reading every file in the current diff, I find no bugs and no blocking issues. The code is well-structured, consistent, and the documentation is thorough. Below are my findings organized by severity.


✅ No blocking issues

Previous review issues were already resolved (or never existed in the current diff)

The two prior reviews flagged several items. Having read all 17 files at their current revision:

Prior finding Status in current code
Broken README link (/en/ segment) Already correctREADME.md:73 links to ../../../docs/guide/snapshot-rollback-clone.md (no /en/)
Version mismatch (requirements.txt vs READMEs) Already consistent — all files say >=0.2.0
Unused API_URL in env.py Not presentenv.py only exports TEMPLATE_ID
Missing defensive stdout checks in 09, 10 Already in place — both files use [0].strip() if ... else ""
Doc listing example drops first page Not present in current code — the docs show single-page API calls, not a while token: loop. The pagination description at line 75 explains the token mechanics without demonstrating multi-page iteration, which is appropriate for the guide format.

💡 Observations (optional polish, not blockers)

1. 08_fork_three_axis.py:33 — Default value inconsistency

leaked = r.logs.stdout[0].strip() if r.logs.stdout else "<empty>"

This uses "<empty>" as the fallback, while every other demo uses "". If stdout were truly empty, the assertion leaked == "False" on line 35 would produce an error message showing "<empty>" rather than "". Very minor — consistent with other demos would be to use "":

leaked = r.logs.stdout[0].strip() if r.logs.stdout else ""

Not a bug (since os.path.exists() always produces stdout), but worth noting for consistency.

2. Documentation guide code snippets — logs.stdout[0] without guard

Both docs/guide/snapshot-rollback-clone.md and docs/zh/guide/snapshot-rollback-clone.md access logs.stdout[0] directly in the clone and rollback snippets (lines 88, 124, 136):

out = sb.run_code("print(open('/tmp/shared.txt').read())").logs.stdout[0]

This is intentional — guide snippets prioritize readability over defensive coding. The runnable demos in CubeAPI/examples/ all use the guarded pattern. No change needed.

3. Copyright year

All files use Copyright (c) 2024 Tencent Inc.. These are new files being added in 2026. This is organization-dependent — some prefer the original project year, others want the current year. Worth verifying with your team's policy.


✅ What looks good

  • Consistent structure: All 11 demos follow the same header and cleanup pattern.
  • Self-contained: Each script is independently runnable with a clear docstring.
  • Thorough coverage: Full surface area — create, list, clone, state preservation, snapshot lifecycle, clone-n, concurrent clone, fork semantics (inheritance+isolation+continuity), rollback, rollback-then-continue, and delete.
  • Defensive patterns: The stdout[0].strip() if stdout else "" guard is used correctly everywhere.
  • Clean pagination: The while True / if not token: break pattern in demos 02, 05, and 11 is correct and consistent.
  • Error-aware cleanup: 10_rollback_then_continue.py wraps delete_snapshot in try/except — a good pattern for rollback demos where snapshot state may be volatile.
  • Guide documentation: Clear mermaid diagram, well-organized sections, and the best-practices section provides practical guidance.
  • README table of contents: Excellent reference table linking demo number to script name and topic.
  • All relative links verified correct: Both English and Chinese READMEs link correctly to their respective guides and SDK source.

Summary

Severity Count Action
Bug 0
Suggestion 1 08_fork_three_axis.py:33 fallback value inconsistency (trivial)
Observation 2 Guardless snippets in docs (intentional), copyright year

Verdict: No blockers found. The PR is ready to merge from a code quality perspective.


@ls-ggg
Copy link
Copy Markdown
Collaborator Author

ls-ggg commented May 27, 2026

/cubebot

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 27, 2026

Claude finished @ls-ggg's task in 1m 15s —— View job


Review: PR #374 — Snapshot/Rollback/Clone Demo Suite & Guide

Check Status
Files reviewed 17 / 17 (+897 / -0)
Bugs found 0
Suggestions 1
Observations 1

✅ No blocking issues

After reading every file in the current diff at HEAD, I find no bugs. The items flagged by prior reviews have been addressed:

Prior finding Current status
Broken README link (/en/ segment) CorrectREADME.md:73 links to ../../../docs/guide/snapshot-rollback-clone.md (no /en/)
Version mismatch (requirements.txt vs READMEs) Consistent — all files require >=0.2.0
Unused API_URL in env.py Not presentenv.py only exports TEMPLATE_ID
Missing defensive stdout checks in 09, 10 In place — both files use [0].strip() if ... else ""
Doc listing example drops first page Not an issue — the guide shows single-page API signatures with accompanying text explaining pagination tokens; it doesn't demonstrate multi-page iteration. The runnable demos handle pagination correctly.

💡 Suggestions

1. 08_fork_three_axis.py:33,39 — Inconsistent fallback value

marker = r.logs.stdout[0].strip() if r.logs.stdout else "<empty>"   # line 26, 33, 39

Uses "<empty>" as the fallback, while every other demo (04, 06, 07, 09, 10) uses "". If stdout were empty, marker would be "<empty>" which the subsequent assert would report in the error message instead of showing the actual (empty) output. Very minor, and the break scenario doesn't arise here since os.path.exists() / open().read() always produce stdout when the file exists. Consider making consistent with the rest of the suite.


📝 Observations

2. Copyright year — 2024 on new files in 2026

All 17 new files use Copyright (c) 2024 Tencent Inc. This may be intentional (matching the project's original copyright year). Worth confirming with your team's policy — some orgs prefer the year of file creation (2026), others use the project's inception year (2024). No change required, just be aware.

3. Doc guide snippets — no stdout guard (intentional)

Both docs/guide/snapshot-rollback-clone.md and docs/zh/guide/snapshot-rollback-clone.md access logs.stdout[0] without a guard. This is appropriate for documentation snippets where brevity matters; the runnable demos all use the guarded pattern. No change needed.


✅ What looks good

  • Consistent structure: All 11 demos follow the same header format (copyright + docstring) and cleanup pattern.
  • Self-contained: Each script is independently runnable with a clear docstring.
  • Thorough coverage: The demos cover the full surface area — create, list, clone, state preservation, snapshot lifecycle, clone-n, concurrent clone, fork semantics (inheritance+isolation+continuity), rollback, rollback-then-continue, and delete.
  • Correct defensive patterns: The [0].strip() if ... else "" guard is used consistently everywhere except 08_fork_three_axis.py (trivial difference in fallback string).
  • Clean pagination: The while True / if not token: break pattern in demos 02, 05, and 11 is correct and handles all pages.
  • Error-aware cleanup: 10_rollback_then_continue.py wraps delete_snapshot in try/except — a good pattern for rollback demos where snapshot state may be volatile.
  • Guide docs: Clear mermaid diagrams, well-organized sections, best-practices section with practical advice.
  • Relative links: All verified correct — both READMEs link to their respective guides and SDK source correctly.

Summary

Severity Count Action
Bug 0
Suggestion 1 08_fork_three_axis.py fallback string consistency (trivial)

Verdict: No blockers found. The PR is ready to merge from a code quality perspective.

@ls-ggg ls-ggg merged commit f9e022a into TencentCloud:master May 27, 2026
9 checks passed
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.

1 participant