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 77.57% 78.06% +0.49%
==================================================================
Files 117 117
Lines 6663 6671 +8
Branches 972 973 +1
==================================================================
+ Hits 5169 5208 +39
+ Misses 1184 1153 -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
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
tkislan
force-pushed
the
feat/dataframe-storage
branch
from
October 8, 2026 12:55
91ffced to
3b63336
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