Skip to content

Commit

Permalink
Code review followups
Browse files Browse the repository at this point in the history
* Use a traditional /home directory
* Combine poetry into python layer
* Rename mounts
* Removed unused imports
* Put tar-fs into devDependencies
  • Loading branch information
jessedearing committed Sep 17, 2024
1 parent 679df68 commit c75df01
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 19 deletions.
11 changes: 4 additions & 7 deletions Dockerfile
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
# == base ======================
FROM buildpack-deps:bookworm AS base
ENV CACHEBUST=2024-09-17
RUN useradd -m -d /project -u 8000 observable-builder && apt update
RUN useradd -m -u 8000 observable-builder && mkdir /project && \
chown 8000:8000 /project && apt update

ENV RUSTUP_HOME=/usr/local/rustup \
CARGO_HOME=/usr/local/cargo \
RUST_VERSION=1.81.0 \
VIRTUAL_ENV=/project/.local/python-venv
ENV PATH=/usr/local/cargo/bin:$VIRTUAL_ENV/bin:/project/.local/bin:$PATH
VIRTUAL_ENV=/home/observable-builder/.local/python-venv
ENV PATH=/usr/local/cargo/bin:$VIRTUAL_ENV/bin:/home/observable-builder/.local/bin:$PATH

# == node ======================
FROM base AS node
Expand All @@ -33,10 +34,7 @@ RUN --mount=type=cache,target=/var/cache/apt,id=framework-runtime-python \
python3-dev \
python3-venv \
pipx

FROM python AS python-poetry
USER 8000
WORKDIR /project
RUN pipx install poetry \
&& python3 -m venv $VIRTUAL_ENV

Expand Down Expand Up @@ -126,7 +124,6 @@ FROM base AS runtime
COPY --from=general-cli . .
COPY --from=node . .
COPY --from=python . .
COPY --from=python-poetry . .
COPY --from=r . .
COPY --from=duckdb . .
COPY --from=rust . .
Expand Down
4 changes: 1 addition & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,8 @@
"dockerode": "^4.0.2",
"glob": "^10.3.12",
"semver": "^7.6.0",
"tar-fs": "^3.0.6",
"tsx": "^4.7.1",
"typescript": "^5.4.3"
},
"dependencies": {
"tar-fs": "^3.0.6"
}
}
2 changes: 1 addition & 1 deletion tests/dataloader-languages.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ describe("Dataloader languages", () => {
],
{
workingDir: "/project/poetry-test",
mounts: [{ host: "./tests/fixtures/poetry-test", container: "/project/poetry-test" }],
hostContainerDirs: [{ host: "./tests/fixtures/poetry-test", container: "/project/poetry-test" }],
},
);
});
Expand Down
13 changes: 5 additions & 8 deletions tests/index.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,9 @@
import { test, before } from "node:test";
import { resolve } from "node:path";
import assert from "node:assert";
import Dockerode from "dockerode";
import { Stream } from "node:stream";
import semverSatisfies from "semver/functions/satisfies";
import { basename } from "node:path";
import tar from "tar-fs";
import { readFileSync } from "node:fs";

export const IMAGE_TAG = "observablehq/framework-runtime:test";

Expand Down Expand Up @@ -91,9 +88,9 @@ before(ensureDocker);

function copyFilesToContainer(
dockerContainer: Dockerode.Container,
files: { host: string; container: string }[],
directories: { host: string; container: string }[],
) {
for (const { host, container } of files) {
for (const { host, container } of directories) {
const tarStream = tar.pack(host);
dockerContainer.putArchive(tarStream, { path: container });
}
Expand All @@ -102,10 +99,10 @@ function copyFilesToContainer(
export async function runCommandInContainer(
command: string[],
{
mounts = [],
hostContainerDirs = [],
workingDir = "/project",
}: {
mounts?: { host: string; container: string}[];
hostContainerDirs?: { host: string; container: string}[];
workingDir?: string;
} = {},
): Promise<{ stdout: string; stderr: string }> {
Expand All @@ -116,7 +113,7 @@ export async function runCommandInContainer(
Cmd: command,
});

copyFilesToContainer(container, mounts);
copyFilesToContainer(container, hostContainerDirs);

const stdout = new StringStream();
const stderr = new StringStream();
Expand Down

0 comments on commit c75df01

Please sign in to comment.