-
Notifications
You must be signed in to change notification settings - Fork 2
chore: Update setup for local development for Deepnote engineers #40
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
base: main
Are you sure you want to change the base?
chore: Update setup for local development for Deepnote engineers #40
Conversation
|
📦 Python package built successfully!
|
📝 WalkthroughWalkthroughThe changes restructure local toolkit development workflows across documentation and Docker configuration. CONTRIBUTING.md documentation is updated to replace the previous Deepnote-specific image workflow with steps for local webapp hot-reload development. A new Dockerfile (jupyter-for-local-hotreload/Dockerfile) is introduced with updated system dependencies, Poetry configuration pointing to /opt/venvs, and /toolkit as the working directory. A corresponding entrypoint script (jupyter-for-local-hotreload/entrypoint.sh) validates toolkit presence, installs dependencies in editable mode, and launches the server. The legacy run-installer.sh script is removed. The flow emphasizes mounting toolkit source into the container for development. Pre-merge checks✅ Passed checks (3 passed)
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.
Actionable comments posted: 2
📜 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.
📒 Files selected for processing (4)
CONTRIBUTING.md(2 hunks)dockerfiles/jupyter-for-local-hotreload/Dockerfile(1 hunks)dockerfiles/jupyter-for-local-hotreload/entrypoint.sh(1 hunks)dockerfiles/jupyter-for-local-hotreload/run-installer.sh(0 hunks)
💤 Files with no reviewable changes (1)
- dockerfiles/jupyter-for-local-hotreload/run-installer.sh
🧰 Additional context used
🪛 Checkov (3.2.334)
dockerfiles/jupyter-for-local-hotreload/Dockerfile
[low] 1-52: Ensure that HEALTHCHECK instructions have been added to container images
(CKV_DOCKER_2)
[low] 1-52: Ensure that a user for the container has been created
(CKV_DOCKER_3)
🪛 Hadolint (2.14.0)
dockerfiles/jupyter-for-local-hotreload/Dockerfile
[warning] 16-16: Pin versions in apt get install. Instead of apt-get install <package> use apt-get install <package>=<version>
(DL3008)
[info] 37-37: Multiple consecutive RUN instructions. Consider consolidation.
(DL3059)
🪛 markdownlint-cli2 (0.18.1)
CONTRIBUTING.md
159-159: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
166-166: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
⏰ 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). (8)
- GitHub Check: Test - Python 3.11
- GitHub Check: Test - Python 3.10
- GitHub Check: Test - Python 3.9
- GitHub Check: Build and push artifacts for Python 3.10
- GitHub Check: Build and push artifacts for Python 3.13
- GitHub Check: Build and push artifacts for Python 3.9
- GitHub Check: Build and push artifacts for Python 3.12
- GitHub Check: Build and push artifacts for Python 3.11
🔇 Additional comments (2)
dockerfiles/jupyter-for-local-hotreload/entrypoint.sh (1)
1-28: Solid implementation of entrypoint logic.Error handling is robust, git configuration for poetry-dynamic-versioning is correct, and using
execto launch the final command is appropriate. Clear error messages guide users when the toolkit source is missing. The script properly passes through CLI arguments.dockerfiles/jupyter-for-local-hotreload/Dockerfile (1)
30-31: Poetry 2.2.0 is available and maintained.Poetry 2.2.0 was released on September 14, 2025, and is published on PyPI. The version is recent and actively maintained, making this explicit pinning appropriate.
| 1. Build the local development image: | ||
| ```bash | ||
| docker build -t deepnote/jupyter-for-local:local -f ./dockerfiles/jupyter-for-local-hotreload/Dockerfile . | ||
| ``` | ||
|
|
||
| ```yml | ||
| jupyter: | ||
| image: 'screwdrivercd/noop-container' | ||
| ``` | ||
| 2. Setup `DEEPNOTE_TOOLKIT_SOURCE_PATH` env variable pointing to folder with toolkit source. This can go either in `.zshrc` (or similar file for your shell) or set per shell session with `export DEEPNOTE_TOOLKIT_SOURCE_PATH=...`. If not set, webapp will try to resolve it to `../deepnote-toolkit` relative to webapp root folder. | ||
|
|
||
| And change `JUPYTER_HOST` variable of executor to point to host machine: | ||
| 3. In the webapp repository, run: | ||
| ```bash | ||
| pnpm dev:app:local-toolkit | ||
| ``` |
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.
Add blank lines around code fences (MD031).
Lines 159–161 and 166–168 need blank lines before and after the fenced code blocks per markdown linting standards.
Apply this diff:
1. Build the local development image:
+
```bash
docker build -t deepnote/jupyter-for-local:local -f ./dockerfiles/jupyter-for-local-hotreload/Dockerfile .
```
+
3. In the webapp repository, run:
+
```bash
pnpm dev:app:local-toolkit
```
+🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
159-159: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
166-166: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
🤖 Prompt for AI Agents
In CONTRIBUTING.md around lines 158 to 168, the two fenced code blocks do not
have blank lines immediately before and after them (MD031); add one blank line
before the opening ```bash and one blank line after the closing ``` for both the
docker build block (lines ~159–161) and the pnpm block (lines ~166–168),
adjusting the surrounding list spacing so the numbered list remains correct and
the markdown linter no longer flags MD031.
| RUN apt-get update && \ | ||
| apt-get install -y openjdk-17-jdk && \ | ||
| apt-get install --no-install-recommends -y \ | ||
| rsync \ | ||
| git \ | ||
| # Required for pymssql | ||
| freetds-dev \ | ||
| # Required for database connectivity through ODBC | ||
| unixodbc-dev \ | ||
| # Required for secure connections (SSL/TLS) | ||
| libssl-dev && \ | ||
| apt-get clean && \ | ||
| rm -rf /var/lib/apt/lists/* | ||
|
|
||
| RUN pip install poetry==2.2.0 | ||
| # Install Poetry and required plugins | ||
| RUN pip install --no-cache-dir poetry==2.2.0 && \ | ||
| poetry self add 'poetry-dynamic-versioning[plugin]>=1.0.0,<2.0.0' | ||
|
|
||
| WORKDIR /deepnote-toolkit | ||
| # Configure Poetry to create virtualenv outside the mounted source directory | ||
| RUN poetry config virtualenvs.path /opt/venvs | ||
|
|
||
| ENV POETRY_NO_INTERACTION=1 \ | ||
| POETRY_VIRTUALENVS_CREATE=1 \ | ||
| POETRY_VIRTUALENVS_IN_PROJECT=0 \ | ||
| POETRY_CACHE_DIR=/tmp/poetry_cache | ||
| # Create toolkit directory (will be mounted over, but needed for initial setup) | ||
| RUN mkdir -p /toolkit /opt/venvs |
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.
🧹 Nitpick | 🔵 Trivial
Consider consolidating RUN instructions to reduce layer count.
The multiple RUN commands (lines 16, 30, 34, 37) can be consolidated into fewer layers. For example, combining apt-get install, Poetry installation, Poetry config, and directory creation into two or three RUN instructions would improve build efficiency and reduce image size.
Consolidation example:
-# Install system dependencies
-RUN apt-get update && \
+# Install system dependencies and Poetry
+RUN apt-get update && \
apt-get install --no-install-recommends -y \
rsync \
git \
- # Required for pymssql
freetds-dev \
- # Required for database connectivity through ODBC
unixodbc-dev \
- # Required for secure connections (SSL/TLS)
libssl-dev && \
apt-get clean && \
- rm -rf /var/lib/apt/lists/*
+ rm -rf /var/lib/apt/lists/* && \
+ pip install --no-cache-dir poetry==2.2.0 && \
+ poetry self add 'poetry-dynamic-versioning[plugin]>=1.0.0,<2.0.0' && \
+ poetry config virtualenvs.path /opt/venvs && \
+ mkdir -p /toolkit /opt/venvsThis reduces from 4+ RUN layers to 1, improving build time and image size.
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Hadolint (2.14.0)
[warning] 16-16: Pin versions in apt get install. Instead of apt-get install <package> use apt-get install <package>=<version>
(DL3008)
[info] 37-37: Multiple consecutive RUN instructions. Consider consolidation.
(DL3059)
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #40 +/- ##
=======================================
Coverage 73.04% 73.04%
=======================================
Files 93 93
Lines 5149 5149
Branches 754 754
=======================================
Hits 3761 3761
Misses 1144 1144
Partials 244 244
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
🚀 Review App Deployment Started
|
This replaces legacy setup we had with
deepnote/deepnote-toolkit-local-hotreloadimage. General idea is same, create empty container and mount source as volume, just updated to work with latest toolkit (as original version was made before major changes we did before open-sourcing)Summary by CodeRabbit
Documentation
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.