Skip to content

Conversation

@FilipPyrek
Copy link
Member

@FilipPyrek FilipPyrek commented Nov 3, 2025

Deepnote Cloud creates init notebooks with pre-defined bash script for installation of dependencies. The problem is that bash doesn't run on Windows by default.

The solution is to replace the pre-defined bash script with cross-platform python script.

Summary by CodeRabbit

  • Bug Fixes

    • Improved cross-platform notebook initialization with enhanced Windows support.
  • New Features

    • Added environment-specific initialization handling for different development environments.

@FilipPyrek FilipPyrek requested a review from a team as a code owner November 3, 2025 18:37
@linear
Copy link

linear bot commented Nov 3, 2025

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 3, 2025

📝 Walkthrough

Walkthrough

Modified the init notebook runner to handle cross-platform compatibility. Added two new constants distinguishing Deepnote cloud and VSCode initialization block contents. When executing notebook blocks on Windows, the system detects if a block contains Deepnote cloud initialization content, replaces it with VSCode-compatible Python content, and proceeds with execution. This allows dependency installation to work on Windows in addition to Linux/macOS environments.

Sequence Diagram

sequenceDiagram
    participant runner as Init Notebook Runner
    participant block as Code Block
    participant logger as Logger

    runner->>block: Retrieve block content
    runner->>runner: Check if Windows platform
    alt Windows & Deepnote Cloud Init Content
        runner->>runner: Replace with VSCode Python content
        runner->>logger: Log content replacement
        rect rgb(240, 248, 255)
            Note over runner: Platform adaptation
        end
    end
    runner->>runner: Execute blockContent (original or replaced)
    alt Execution Success
        runner->>logger: Log success
    else Execution Error
        runner->>logger: Log error & continue
    end
Loading

Pre-merge checks

✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: fixing init notebook execution on Windows by replacing bash scripts with cross-platform Python equivalents.
Linked Issues check ✅ Passed The PR implements the core objective from BLU-5107: replacing bash init scripts with cross-platform Python code to support Windows execution.
Out of Scope Changes check ✅ Passed All changes in deepnoteInitNotebookRunner.node.ts directly address the Windows compatibility issue for init notebook execution; no out-of-scope modifications detected.

Comment @coderabbitai help to get the list of available commands and usage tips.

@FilipPyrek FilipPyrek requested a review from saltenasl November 3, 2025 18:37
@codecov
Copy link

codecov bot commented Nov 3, 2025

Codecov Report

❌ Patch coverage is 25.00000% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 73%. Comparing base (b41af0f) to head (c8086dc).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...ebooks/deepnote/deepnoteInitNotebookRunner.node.ts 25% 6 Missing ⚠️
Additional details and impacted files
@@          Coverage Diff          @@
##            main    #166   +/-   ##
=====================================
- Coverage     73%     73%   -1%     
=====================================
  Files        574     574           
  Lines      46677   46684    +7     
  Branches    5512    5513    +1     
=====================================
+ Hits       34213   34215    +2     
- Misses     10652   10657    +5     
  Partials    1812    1812           
Files with missing lines Coverage Δ
...ebooks/deepnote/deepnoteInitNotebookRunner.node.ts 8% <25%> (+1%) ⬆️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 90bfefd and 36b83ed.

📒 Files selected for processing (1)
  • src/notebooks/deepnote/deepnoteInitNotebookRunner.node.ts (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/notebooks/deepnote/deepnoteInitNotebookRunner.node.ts (1)
src/platform/logging/index.ts (1)
  • logger (35-48)
⏰ 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). (1)
  • GitHub Check: Build & Test
🔇 Additional comments (1)
src/notebooks/deepnote/deepnoteInitNotebookRunner.node.ts (1)

272-285: Code is correct; brittleness concern doesn't apply.

The exact string match is sound because DEEPNOTE_CLOUD_INIT_NOTEBOOK_BLOCK_CONTENT is a fixed constant from a single location, properly trimmed, with no competing bash-based init blocks in the codebase. The Windows-specific replacement logic is appropriate.

@saltenasl
Copy link
Member

@coderabbitai resolve

@saltenasl saltenasl enabled auto-merge (squash) November 3, 2025 19:16
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 3, 2025

✅ Actions performed

Comments resolved and changes approved.

@saltenasl saltenasl disabled auto-merge November 3, 2025 19:17
@saltenasl saltenasl merged commit ed71529 into main Nov 3, 2025
11 of 13 checks passed
@saltenasl saltenasl deleted the fpyrek/BLU-5107-1762181011 branch November 3, 2025 19:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants