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 @@
## feat/artifacts-store-dataframe #131 +/- ##
==================================================================
+ Coverage 78.01% 78.77% +0.75%
==================================================================
Files 118 118
Lines 6783 6878 +95
Branches 989 1002 +13
==================================================================
+ Hits 5292 5418 +126
+ Misses 1181 1150 -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
|
4 of 6 tasks
tkislan
force-pushed
the
feat/dataframe-storage
branch
from
October 8, 2026 12:55
91ffced to
3b63336
Compare
A `post_run_cell` hook stores the DataFrame a cell returns, under the name the
webapp passes in `execute_request` metadata (`deepnote.dataframeStorage` =
`{name, blockId}`), as `artifacts.store_dataframe` would, and records the block in
the manifest as `block_id`. `DEEPNOTE_DATAFRAME_STORAGE_ENABLED` switches the hook
on; it is registered as one more guarded step in `init_deepnote_runtime()`.
Unlike the function, the hook never raises and never prints into the cell: it
skips helper executions, failed cells, previews and PySpark frames and read-only
mounts, makes SIGINT raise during the write, and reports failures to the webapp
once per kernel session per errno or exception type.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B441SpUqRn7QTzcnWxjNa5
…manifest write - The hook raises a SIGINT held during a manifest write that also fails, instead of reporting the write's error and leaving the reply ok, which let "Run all" go on after Stop. - Use the spark_without_ansi_mode fixture for the pandas-on-Spark result. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FhzfrytCUojy8Vw1gbX3Bx
tkislan
force-pushed
the
feat/dataframe-storage
branch
from
October 12, 2026 19:40
3b63336 to
0d1a11a
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 and one manifest field.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/dataframe_storage.py: the hook, registered as one more guarded step ininit_deepnote_runtime(), the metadata reader and the error reports.deepnote_toolkit/dataframe_storage_manifest.py:write_manifesttakes an optionalblock_id.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 only runtime typetoolkit/errorsaccepts), 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.signal.default_int_handleris installed for the write, because a cell with top-level await runspost_run_cellunder a handler that only queues the interrupt. The reply is already decided by then, so the hook setsresult.error_in_execand callsshowtraceback(), which makes the executor stop its queue.nameandblockIdfrom request metadata must match[A-Za-z0-9_-]{1,128}, because the name becomes a directory.result.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.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/dataframe_storage tests/unit/test_runtime_initialization.py -p no:randomly: 229 passed (the 141 tests of feat(artifacts): add store_dataframe and delete_dataframe #135 and 88 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: 1512 passed, 4 skipped, 1 failed. The failure istest_redshift_dialect.py::test_redshift_distribution_matches_python_version: the venv has two redshift packages installed, and it fails identically on a cleanorigin/main.🤖 Generated with Claude Code
https://claude.ai/code/session_01B441SpUqRn7QTzcnWxjNa5