Skip to content

Commit eccfa17

Browse files
committed
python: make restore_fn optional and symmetric with save_fn
Signed-off-by: Cong Wang <cwang@multikernel.io>
1 parent 5caca6b commit eccfa17

3 files changed

Lines changed: 51 additions & 13 deletions

File tree

python/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -571,7 +571,7 @@ Checkpoint.restore("my-snapshot", restore_fn=lambda data: rebuild(data))
571571
|--------|-------------|
572572
| `cp.save(name, store=None)` | Persist checkpoint to disk |
573573
| `Checkpoint.load(name, store=None)` | Load from disk |
574-
| `Checkpoint.restore(name, restore_fn, store=None)` | Load and call restore_fn with app_state |
574+
| `Checkpoint.restore(name, restore_fn=None, store=None)` | Load; if app_state was saved (via save_fn) call restore_fn with it. restore_fn and save_fn must both be present or both absent. |
575575
| `Checkpoint.list(store=None)` | List saved checkpoint names |
576576
| `Checkpoint.delete(name, store=None)` | Delete a saved checkpoint |
577577

python/src/sandlock/_sdk.py

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -861,18 +861,25 @@ def load(cls, name: str, *, store: Path | str | None = None) -> "Checkpoint":
861861
def restore(
862862
cls,
863863
name: str,
864-
restore_fn: "Callable[[bytes], None]",
864+
restore_fn: "Callable[[bytes], None] | None" = None,
865865
*,
866866
store: "Path | str | None" = None,
867867
) -> "Checkpoint":
868-
"""Load a checkpoint and pass its app state to restore_fn.
868+
"""Load a checkpoint and, if it carries app state, pass it to restore_fn.
869869
870-
Convenience for ``load()`` + calling ``restore_fn(cp.app_state)``.
870+
Convenience for ``load()`` plus calling ``restore_fn(cp.app_state)``.
871+
872+
``restore_fn`` mirrors ``save_fn`` on ``Sandbox.checkpoint``: a checkpoint
873+
taken without ``save_fn`` has no app state and is restored without
874+
``restore_fn``; a checkpoint taken with ``save_fn`` carries app state and
875+
must be restored with ``restore_fn``. Supplying exactly one of the pair
876+
is an error.
871877
872878
Args:
873879
name: Checkpoint name.
874-
restore_fn: Callback that receives the saved application-level
875-
state bytes. Use this to rebuild state that ptrace can't
880+
restore_fn: Optional callback receiving the saved application-level
881+
state bytes. Required if (and only if) the checkpoint was taken
882+
with ``save_fn``. Use it to rebuild state that ptrace cannot
876883
capture (caches, session data, etc.).
877884
store: Storage root. Defaults to ``~/.sandlock/checkpoints/``.
878885
@@ -881,16 +888,26 @@ def restore(
881888
882889
Raises:
883890
FileNotFoundError: If the checkpoint does not exist.
884-
ValueError: If the checkpoint has no app_state.
891+
ValueError: If the checkpoint has app_state but no restore_fn was
892+
given, or a restore_fn was given but the checkpoint has no
893+
app_state.
885894
"""
886895
cp = cls.load(name, store=store)
887896
state = cp.app_state
888-
if state is None:
897+
has_state = state is not None
898+
has_fn = restore_fn is not None
899+
if has_state and not has_fn:
900+
raise ValueError(
901+
f"Checkpoint {name!r} has app_state; pass restore_fn to "
902+
"consume it (it was created with save_fn)"
903+
)
904+
if has_fn and not has_state:
889905
raise ValueError(
890-
f"Checkpoint {name!r} has no app_state "
891-
"was it created with save_fn?"
906+
f"Checkpoint {name!r} has no app_state; restore_fn was given "
907+
"but there is nothing to pass (was it created with save_fn?)"
892908
)
893-
restore_fn(state)
909+
if has_fn:
910+
restore_fn(state)
894911
return cp
895912

896913
@classmethod

python/tests/test_checkpoint.py

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -146,12 +146,33 @@ def test_restore_returns_checkpoint(self, running_sandbox, tmp_dir):
146146
assert isinstance(result, Checkpoint)
147147
assert result.name == "ret-test"
148148

149-
def test_restore_no_app_state_raises(self, running_sandbox, tmp_dir):
149+
def test_restore_no_app_state_no_fn_succeeds(self, running_sandbox, tmp_dir):
150+
# save_fn absent at checkpoint, restore_fn absent at restore: both
151+
# absent is the symmetric, valid case and must not raise.
150152
cp = running_sandbox.checkpoint()
151153
cp.save("no-app", store=tmp_dir)
152154

155+
result = Checkpoint.restore("no-app", store=tmp_dir)
156+
assert isinstance(result, Checkpoint)
157+
assert result.app_state is None
158+
159+
def test_restore_fn_without_app_state_raises(self, running_sandbox, tmp_dir):
160+
# restore_fn given but the checkpoint has no app_state: a one-sided
161+
# pairing, which is rejected.
162+
cp = running_sandbox.checkpoint()
163+
cp.save("no-app-fn", store=tmp_dir)
164+
153165
with pytest.raises(ValueError, match="no app_state"):
154-
Checkpoint.restore("no-app", lambda d: None, store=tmp_dir)
166+
Checkpoint.restore("no-app-fn", lambda d: None, store=tmp_dir)
167+
168+
def test_restore_app_state_without_fn_raises(self, running_sandbox, tmp_dir):
169+
# app_state present but restore_fn omitted: the other one-sided pairing,
170+
# also rejected.
171+
cp = running_sandbox.checkpoint(save_fn=lambda: b"state")
172+
cp.save("app-no-fn", store=tmp_dir)
173+
174+
with pytest.raises(ValueError, match="has app_state"):
175+
Checkpoint.restore("app-no-fn", store=tmp_dir)
155176

156177
def test_restore_nonexistent_raises(self, tmp_dir):
157178
with pytest.raises(FileNotFoundError):

0 commit comments

Comments
 (0)