-
Notifications
You must be signed in to change notification settings - Fork 0
feat: add kernel metadata fallback #32
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Andy Jakubowski <hello@andyjakubowski.com>
GRN-4810 feat: add kernel metadata fallback
Fallback kernel metadata to use until the .deepnote file format includes kernel information. So that we don’t show the kernel selection dialog on .deepnote file open. |
📝 WalkthroughWalkthrough
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Caller
participant T as transformDeepnoteYamlToNotebookContent
participant Y as Deepnote YAML
participant F as kernelMetadataFallback
C->>T: transform(yaml)
T->>Y: Read metadata from YAML
alt Kernel info present in YAML
T->>T: Use YAML kernel metadata
else No kernel info in YAML
T->>F: Import and read fallback metadata
T->>T: Merge fallback into notebook metadata
end
T-->>C: Return notebook content with metadata
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #32 +/- ##
=======================================
Coverage ? 37.29%
=======================================
Files ? 13
Lines ? 244
Branches ? 26
=======================================
Hits ? 91
Misses ? 153
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/kernel-metadata-fallback.ts(1 hunks)src/transform-deepnote-yaml-to-notebook-content.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/transform-deepnote-yaml-to-notebook-content.ts (1)
src/kernel-metadata-fallback.ts (1)
kernelMetadataFallback(2-20)
🪛 GitHub Actions: CI
src/kernel-metadata-fallback.ts
[error] 3-3: Unknown word (kernelspec)
[error] 4-4: Unknown word (ipykernel)
[error] 10-10: Unknown word (ipython)
[error] 16-16: Unknown word (nbconvert)
[error] 17-17: Unknown word (pygments)
[error] 17-17: Unknown word (ipython)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Lint & Format
- GitHub Check: Audit - Production
- GitHub Check: Audit - All
- GitHub Check: build
- GitHub Check: check_release
🔇 Additional comments (2)
src/kernel-metadata-fallback.ts (1)
3-19: Pipeline spell-check failures are false positives.Terms like
kernelspec,ipykernel,ipython,nbconvert, andpygmentsare legitimate Jupyter/Python identifiers. Add them to your spell-check dictionary or ignore list.src/transform-deepnote-yaml-to-notebook-content.ts (1)
5-5: LGTM.Import and spread correctly provide fallback kernel metadata.
Also applies to: 46-46
Artmann
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
![]()
|
@andyjakubowski build and spellchecks failing |
Signed-off-by: Andy Jakubowski <hello@andyjakubowski.com>
83182b2
Signed-off-by: Andy Jakubowski <hello@andyjakubowski.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
jupyterlab_deepnote/tests/test_handlers.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
jupyterlab_deepnote/tests/test_handlers.py (1)
jupyterlab_deepnote/handlers.py (2)
RouteHandler(10-59)get(15-59)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Spell Check
- GitHub Check: check_release
- GitHub Check: build
🔇 Additional comments (4)
jupyterlab_deepnote/tests/test_handlers.py (4)
21-24: LGTM!Correctly validates 400 for missing path parameter.
45-52: Same mock concern as permission denied test.Logic correct for 500 on unexpected error. Same verification needed for mock approach.
27-32: Ignore inconsistency warning; test is correct
contents_manager.get raises tornado.web.HTTPError(404), not FileNotFoundError, so the FileNotFoundError catch is never hit and the generic exception yields a 500—exactly what the test asserts.Likely an incorrect or invalid review comment.
35-42: Mocking contents_manager with patch.object(create=True) is correct Instances inherit the class attribute, so the test properly triggers the 403 path. No changes needed.
The goal of this PR is to set kernel metadata on .deepnote files, so you don’t get that Select Kernel dialog every time you open the .deepnote file.
Fallback kernel metadata to use until the .deepnote file format includes kernel information.
Signed-off-by: Andy Jakubowski hello@andyjakubowski.com
Summary by CodeRabbit
New Features
Bug Fixes
Tests