Skip to content

Commit 3eda250

Browse files
committed
refactor: replace ChunkInterrupted snapshot copies with live-view properties
Drop the .copy() snapshot of call.partial_frame and the stored partial_response from ChunkInterrupted.__init__. Instead, partial_frame and partial_response are now @Property delegates to self.call when available, falling back to empty/None when call is None. This avoids copying potentially large DataFrames on every ChunkInterrupted raise — the primary usage pattern is exc.call.resume(), not inspecting the partial data on the exception itself. Pickling still works: __getstate__ snapshots the current partial_frame and partial_response into private backing attributes that __setstate__ restores, so the degraded (call=None) unpickled form retains the data that was available at serialization time.
1 parent 5686d30 commit 3eda250

2 files changed

Lines changed: 74 additions & 46 deletions

File tree

dataretrieval/ogc/interruptions.py

Lines changed: 52 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -65,14 +65,14 @@ class ChunkInterrupted(DataRetrievalError):
6565
total_chunks : int
6666
Total sub-requests in the plan.
6767
partial_frame : pandas.DataFrame
68-
Combined frame of work completed by the moment this exception
69-
was raised. Snapshot at raise time — does NOT advance on a
70-
later ``call.resume()`` (use ``exc.call.partial_frame`` for
71-
the live view).
68+
Live view of work completed so far — delegates to
69+
``call.partial_frame``. Advances as ``call.resume()`` makes
70+
further progress. Returns an empty ``DataFrame`` when
71+
``call`` is ``None`` (degraded/unpickled state).
7272
partial_response : httpx.Response or None
73-
Raw aggregate response covering the completed sub-requests at
74-
raise time; ``None`` if nothing had completed yet. Same snapshot
75-
semantics as ``partial_frame``. (Raw, not finalized — use
73+
Live view of the raw aggregate response — delegates to
74+
``call.partial_response``. ``None`` when ``call`` is ``None``
75+
or nothing has completed yet. (Raw, not finalized — use
7676
``exc.call.resume()`` for the finalized ``(df, metadata)`` result.)
7777
7878
Examples
@@ -128,16 +128,32 @@ def __init__(
128128
self.total_chunks = total_chunks
129129
self.call = call
130130
self.retry_after = retry_after
131-
# Snapshot partial state at raise time so the exception's view stays
132-
# stable across later ``call.resume()`` advances (the live view is on
133-
# ``call.partial_frame`` / ``.partial_response``). ``.copy()`` guards
134-
# the single-chunk fast path, where the frame may be returned verbatim.
135-
if call is None:
136-
self.partial_frame: pd.DataFrame = pd.DataFrame()
137-
self.partial_response: httpx.Response | None = None
138-
else:
139-
self.partial_frame = call.partial_frame.copy()
140-
self.partial_response = call.partial_response
131+
132+
@property
133+
def partial_frame(self) -> pd.DataFrame:
134+
"""Live view of work completed so far — delegates to ``call.partial_frame``.
135+
136+
Returns an empty ``DataFrame`` when ``call`` is ``None`` (degraded /
137+
unpickled state), unless a pickled snapshot was restored via
138+
``__setstate__``.
139+
"""
140+
if self.call is not None:
141+
return self.call.partial_frame
142+
# Degraded / unpickled: return the snapshot captured at pickle time,
143+
# or an empty frame if no snapshot exists (hand-constructed exc).
144+
return getattr(self, "_pickled_partial_frame", pd.DataFrame())
145+
146+
@property
147+
def partial_response(self) -> httpx.Response | None:
148+
"""Live view of the raw aggregate response.
149+
150+
Delegates to ``call.partial_response``.
151+
Returns ``None`` when ``call`` is ``None`` (degraded / unpickled state)
152+
and no pickled snapshot exists.
153+
"""
154+
if self.call is not None:
155+
return self.call.partial_response
156+
return getattr(self, "_pickled_partial_response", None)
141157

142158
def __getstate__(self) -> dict[str, Any]:
143159
# Drop the live ChunkedCall before pickling: its ``.fetch`` is an
@@ -146,7 +162,25 @@ def __getstate__(self) -> dict[str, Any]:
146162
# The degraded ``call=None`` form keeps the counts, retry hint, and
147163
# partial frame / response; only ``.resume()`` is lost (cross-process
148164
# resume was never possible anyway).
149-
return {**super().__getstate__(), "call": None}
165+
#
166+
# Since ``partial_frame`` and ``partial_response`` are now live
167+
# properties (delegating to ``self.call``), we must snapshot them
168+
# here so the pickled state includes the data that was available
169+
# at serialization time.
170+
state = {**super().__getstate__(), "call": None}
171+
state["_pickled_partial_frame"] = self.partial_frame.copy()
172+
state["_pickled_partial_response"] = self.partial_response
173+
return state
174+
175+
def __setstate__(self, state: dict[str, Any] | None) -> None:
176+
state = state or {}
177+
# Restore the pickled partial snapshots into private backing attrs
178+
# that the properties will fall back to when call is None.
179+
self._pickled_partial_frame = state.pop(
180+
"_pickled_partial_frame", pd.DataFrame()
181+
)
182+
self._pickled_partial_response = state.pop("_pickled_partial_response", None)
183+
super().__setstate__(state)
150184

151185

152186
class QuotaExhausted(ChunkInterrupted):

tests/waterdata_chunking_test.py

Lines changed: 22 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -954,13 +954,11 @@ async def fetch(args):
954954
assert err.partial_response is not None
955955

956956

957-
def test_partial_frame_snapshot_stable_across_resume():
958-
"""``exc.partial_frame`` / ``exc.partial_response`` snapshot the
959-
state at raise time. Calling ``exc.call.resume()`` advances the
960-
underlying ``ChunkedCall`` but must NOT mutate the snapshot on
961-
the exception — otherwise a diagnostic that reads
962-
``exc.partial_frame`` after a resume sees post-resume state under
963-
a name that promises pre-resume state."""
957+
def test_partial_frame_is_live_view_across_resume():
958+
"""``exc.partial_frame`` is now a live view delegating to
959+
``exc.call.partial_frame``. Calling ``exc.call.resume()`` advances the
960+
underlying ``ChunkedCall`` and ``exc.partial_frame`` reflects that — it
961+
is no longer a frozen snapshot."""
964962
state = {"i": 0, "blow_up": True}
965963

966964
async def fetch(args):
@@ -977,27 +975,24 @@ async def fetch(args):
977975
with pytest.raises(QuotaExhausted) as excinfo:
978976
decorated({"sites": ["S1" * 10, "S2" * 10, "S3" * 10, "S4" * 10, "S5" * 10]})
979977
err = excinfo.value
980-
snapshot_rows = len(err.partial_frame)
981-
assert snapshot_rows > 0 # two chunks worth of data captured
978+
pre_resume_rows = len(err.partial_frame)
979+
assert pre_resume_rows > 0 # two chunks worth of data captured
982980

983-
# Resume; the live view on .call grows.
981+
# Resume; the live view on .call grows — and so does exc.partial_frame.
984982
state["blow_up"] = False
985983
err.call.resume()
986-
assert len(err.call.partial_frame) > snapshot_rows
984+
assert len(err.call.partial_frame) > pre_resume_rows
987985

988-
# The exception's snapshot must NOT advance.
989-
assert len(err.partial_frame) == snapshot_rows
986+
# The exception's partial_frame IS the live view — it advances too.
987+
assert len(err.partial_frame) == len(err.call.partial_frame)
990988

991989

992-
def test_partial_frame_snapshot_is_a_copy_when_single_chunk():
993-
"""``_combine_chunk_frames`` returns ``non_empty[0]`` verbatim on
994-
its single-frame fast path. ``ChunkInterrupted.__init__`` must
995-
therefore defensively ``.copy()`` so an in-place mutation of the
996-
underlying chunk frame (e.g. user diagnostic code adding a
997-
column on the live view) doesn't leak through the snapshot.
998-
Companion to ``test_partial_frame_snapshot_stable_across_resume``,
999-
which uses ≥2 completed chunks and so goes through
1000-
``pd.concat`` (which already produces a fresh frame)."""
990+
def test_partial_frame_is_live_view_when_single_chunk():
991+
"""``partial_frame`` is a live view delegating to ``call.partial_frame``.
992+
When a single chunk completes, ``partial_frame`` still reflects the
993+
current state of ``call.partial_frame`` (which recomputes on each
994+
access). This verifies the live-view semantics even on the single-
995+
chunk fast path."""
1001996
state = {"i": 0, "blow_up": True}
1002997

1003998
async def fetch(args):
@@ -1020,12 +1015,11 @@ async def fetch(args):
10201015
err = excinfo.value
10211016
assert err.completed_chunks == 1
10221017

1023-
snapshot_cols = list(err.partial_frame.columns)
1024-
# Mutate the underlying chunk in place — the snapshot must NOT
1025-
# reflect the mutation.
1026-
err.call._chunks[0][0]["extra"] = 0
1027-
assert list(err.partial_frame.columns) == snapshot_cols
1028-
assert "extra" not in err.partial_frame.columns
1018+
# The live view delegates to call.partial_frame; verify it's
1019+
# accessible and reflects the call's current state.
1020+
assert not err.partial_frame.empty
1021+
assert list(err.partial_frame.columns) == ["sites"]
1022+
assert err.partial_frame.equals(err.call.partial_frame)
10291023

10301024

10311025
def test_combine_chunk_responses_returns_independent_headers():

0 commit comments

Comments
 (0)