Repository navigation
Conversation
|
📦 Python package built successfully!
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #131 +/- ##
==========================================
+ Coverage 78.43% 79.16% +0.72%
==========================================
Files 121 122 +1
Lines 6920 7013 +93
Branches 1005 1017 +12
==========================================
+ Hits 5428 5552 +124
+ Misses 1182 1151 -31
Partials 310 310
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
|
🚀 Review App Deployment Started
|
tkislan
force-pushed
the
feat/dataframe-storage
branch
from
October 8, 2026 12:55
91ffced to
3b63336
Compare
tkislan
force-pushed
the
feat/dataframe-storage
branch
2 times, most recently
from
October 14, 2026 09:51
0d1a11a to
b832280
Compare
When the platform asks for it, a post_run_cell hook stores a Python or SQL
block's DataFrame result as the block's frame, in
.deepnote/dataframes/<notebookId>/<blockId>/, where the webapp's dataframe
reader serves block frames from. The frame is written by the same writer as
artifacts.store_dataframe, so it has the same Arrow file and manifest.
The webapp asks per execution through the execute_request metadata
deepnote.dataframeStorage = {notebookId, blockId}, and the hook runs only
when DEEPNOTE_DATAFRAME_STORAGE_ENABLED is on. It skips helper executions,
failed or interrupted cells, SQL query previews, PySpark and pandas-on-Spark
frames, non-DataFrame results and read-only mounts.
The hook never raises into the cell and suppresses warnings. A failed write
is reported to the webapp as TOOLKIT_RUNTIME_ERROR with code
DATAFRAME_STORAGE_WRITE_FAILED, once per kernel session per errno or
exception type, carrying the exception type but never its message. A Stop
during the write fails the execution with KeyboardInterrupt, so the
executor stops its queue.
store_frame now takes the frame's directory, and store_dataframe and the
hook share is_storable_frame.
tkislan
force-pushed
the
feat/dataframe-storage
branch
from
November 10, 2026 17:19
5e6b392 to
dee4404
Compare
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
@coderabbitai ignore
Summary
Phase 2 of the "Full Dataframe storage" RFC (§3.6.4): storing a block's result automatically. Stacked on #135, which adds
artifacts.store_dataframeand the writer; this PR only adds the hook on top of it, so review that one first. The diff against it is the hook, its tests, one manifest field and the query-preview test builders, now shared.When a Python or SQL block has a storage setting, the webapp puts the setting's frame name in the
execute_requestmetadata (deepnote.dataframeStorage={name, blockId}). Apost_run_cellhook then stores the cell's result under that name, as if the block ended withartifacts.store_dataframe(result, name), and records the block in the manifest asblock_id. A later call tostore_dataframewrites noblock_id, so a frame that code stored last belongs to no block.deepnote_toolkit/artifacts/dataframe_storage.py: the hook, registered as one more guarded step ininit_deepnote_runtime(), the metadata reader, the error reports, andis_storable_frame, which decides which results are stored.deepnote_toolkit/artifacts/dataframe_storage_manifest.py:Manifesthas an optionalblock_idfield, whichwrite_manifesttakes and leaves out of the JSON when it is absent.DEEPNOTE_DATAFRAME_STORAGE_ENABLEDswitches the hook on. It does not affectstore_dataframe.Out of scope (other repos): the block setting, webapp and executor metadata, the feature flag, the read endpoint, and the cleanup that deletes a block's frame when its setting is cleared.
Behaviour worth reviewing
TOOLKIT_RUNTIME_ERRORwithcode: DATAFRAME_STORAGE_WRITE_FAILED(the typetoolkit/errorssends to Sentry, grouped bycode), withrequestslike the other userpod-API callers. An error status from the webapp is logged to the file logger as well. The report carries the exception type, never its message, because pyarrow quotes the failing value.KeyboardInterrupt, print "Error in callback" and keep the cell successful, so the hook setsresult.error_in_execand callsshowtraceback(). ipykernel builds the reply from the result after the hook, so the reply carriesKeyboardInterruptand the executor stops its queue. No SIGINT handler is installed: a cell with top-level await runspost_run_cellunder a handler that only queues the interrupt, so Stop during that write is lost and the run goes on, as with any blocking code at the end of such a cell.nameandblockIdfrom request metadata must match[A-Za-z0-9_-]{1,128}, because the name becomes a directory.EROFSbefore anything is converted), SQL query previews, PySpark and pandas-on-Spark frames, non-DataFrame results, failed or interrupted cells, and helper executions. Helpers are recognised byresult.info.store_history, notexecution_count: IPython 9 setsexecution_counteven when no history is stored, so the RFC's earlier "noexecution_count" premise only holds on IPython 8.1,true,yes,on(case-insensitive), the same set as_to_boolindeepnote_core/config/loader.py.Changes since the previous revision of this PR
The RFC moved from per-block storage to named frames, and its first part (the function) moved to #135:
deepnote_dataframes/<name>/, not.deepnote/dataframes/<notebookId>/<blockId>/.{name, blockId}.enabledandnotebookIdare gone: the webapp adds the field only for executions that should store._sigint_raises) is gone, as is feat(artifacts): add store_dataframe and delete_dataframe #135's deferral of Stop during the manifest update.block_idis a field of it.store_frametakes the typed frame instead of a kind, so the hook checks results withis_storable_frameinstead ofclassify_frame. Cleanup checkspreviouswhere it unlinks it. The hook tests follow feat(artifacts): add store_dataframe and delete_dataframe #135's test cuts: the case that asserted whatever pyarrow does is gone, and one of the five unserializable frames is left.Not verified here
set_notebook_path()'s precedence, which useshome_dirwithout a/worksuffix. Confirm on a pod that it equals the project mount.DEEPNOTE_DATAFRAME_STORAGE_ENABLEDvalue comes from the sandbox manager, which appends a project's own variables after the system ones, so a project can override it (RFC §3.6.2). Not fixed here.Test plan
poetry run python -m pytest tests/unit/artifacts tests/unit/test_runtime_initialization.py: 182 passed (the 107 tests of feat(artifacts): add store_dataframe and delete_dataframe #135 and 75 for the hook)raise_for_status, warning suppression, read-only andEROFShandling,block_id, registration at startup); every break failed a testblack,isort,flake8clean on the touched files;mypy deepnote_toolkit/cleantests/unit: 1461 passed, 9 skipped🤖 Generated with Claude Code
https://claude.ai/code/session_01B441SpUqRn7QTzcnWxjNa5