Skip to content
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.

Sign up for GitHub

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

fix: eph env + improve docker images to run in userspace #32017

Merged
merged 5 commits into from
Jan 29, 2025
Merged

fix: eph env + improve docker images to run in userspace #32017

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
062e935
feat: improve docker images to support running in userspace
mistercrunch Jan 28, 2025
7f33981
use root for translations
mistercrunch Jan 29, 2025
6b9dbb2
remove $
mistercrunch Jan 29, 2025
e7c5a39
back to root
mistercrunch Jan 29, 2025
3bd1da2
more tweaks
mistercrunch Jan 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,3 +124,4 @@ docker/*local*
# Jest test report
test-report.html
superset/static/stats/statistics.html
.aider*
71 changes: 36 additions & 35 deletions Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,7 @@ COPY superset-frontend /app/superset-frontend
FROM superset-node-ci AS superset-node

# Build the frontend if not in dev mode
RUN --mount=type=cache,target=/app/superset-frontend/.temp_cache \
--mount=type=cache,target=/root/.npm \
RUN --mount=type=cache,target=$/root/.npm \
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here caching the cache mount was preventing the docker-compose-level mount from hooking up the cache to the host. Getting a nice perf boost by letting the host mount do what it's supposed to.

if [ "$DEV_MODE" = "false" ]; then \
echo "Running 'npm run ${BUILD_CMD}'"; \
npm run ${BUILD_CMD}; \
Expand All @@ -100,21 +99,14 @@ RUN if [ "$BUILD_TRANSLATIONS" = "true" ]; then \
# Base python layer
######################################################################
FROM python:${PY_VER} AS python-base
ARG BUILD_TRANSLATIONS="false" # Include translations in the final build
ENV BUILD_TRANSLATIONS=${BUILD_TRANSLATIONS}
ARG DEV_MODE="false" # Skip frontend build in dev mode
ENV DEV_MODE=${DEV_MODE}

ENV LANG=C.UTF-8 \
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LANG/ LC_ALL are not needed anymore as of 3.9 I believe. Other ENVs we don't need in this layer. Turns out ENV don't carry through a layer with a different base and led to some confusion on my part. As it was ENV would not carry through to the final layers, but they do when located in python-common

LC_ALL=C.UTF-8 \
SUPERSET_ENV=production \
FLASK_APP="superset.app:create_app()" \
PYTHONPATH="/app/pythonpath" \
SUPERSET_HOME="/app/superset_home" \
SUPERSET_PORT=8088

ARG SUPERSET_HOME="/app/superset_home"
ENV SUPERSET_HOME=${SUPERSET_HOME}

RUN useradd --user-group -d ${SUPERSET_HOME} -m --no-log-init --shell /bin/bash superset
RUN mkdir -p $SUPERSET_HOME
RUN useradd --user-group -d ${SUPERSET_HOME} -m --no-log-init --shell /bin/bash superset \
&& chmod -R 1777 $SUPERSET_HOME \
&& chown -R superset:superset $SUPERSET_HOME

# Some bash scripts needed throughout the layers
COPY --chmod=755 docker/*.sh /app/docker/
Expand All @@ -125,19 +117,6 @@ RUN pip install --no-cache-dir --upgrade uv
RUN uv venv /app/.venv
ENV PATH="/app/.venv/bin:${PATH}"

# Install Playwright and optionally setup headless browsers
ARG INCLUDE_CHROMIUM="true"
ARG INCLUDE_FIREFOX="false"
RUN --mount=type=cache,target=/root/.cache/uv\
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moving this section to the python-common layer that's used for the app itself as the python/backend translation builder layer does not need this

if [ "$INCLUDE_CHROMIUM" = "true" ] || [ "$INCLUDE_FIREFOX" = "true" ]; then \
uv pip install playwright && \
playwright install-deps && \
if [ "$INCLUDE_CHROMIUM" = "true" ]; then playwright install chromium; fi && \
if [ "$INCLUDE_FIREFOX" = "true" ]; then playwright install firefox; fi; \
else \
echo "Skipping browser installation"; \
fi

######################################################################
# Python translation compiler layer
######################################################################
Expand All @@ -159,13 +138,20 @@ RUN if [ "$BUILD_TRANSLATIONS" = "true" ]; then \
# Python APP common layer
######################################################################
FROM python-base AS python-common

ENV SUPERSET_HOME="/app/superset_home" \
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these now carry through to the 3 final images

HOME="/app/superset_home" \
SUPERSET_ENV="production" \
FLASK_APP="superset.app:create_app()" \
PYTHONPATH="/app/pythonpath" \
SUPERSET_PORT="8088"

# Copy the entrypoints, make them executable in userspace
COPY --chmod=755 docker/entrypoints /app/docker/entrypoints

WORKDIR /app
# Set up necessary directories and user
RUN mkdir -p \
${SUPERSET_HOME} \
${PYTHONPATH} \
superset/static \
requirements \
Expand All @@ -174,6 +160,19 @@ RUN mkdir -p \
requirements \
&& touch superset/static/version_info.json

# Install Playwright and optionally setup headless browsers
ARG INCLUDE_CHROMIUM="true"
ARG INCLUDE_FIREFOX="false"
RUN --mount=type=cache,target=${SUPERSET_HOME}/.cache/uv \
if [ "$INCLUDE_CHROMIUM" = "true" ] || [ "$INCLUDE_FIREFOX" = "true" ]; then \
uv pip install playwright && \
playwright install-deps && \
if [ "$INCLUDE_CHROMIUM" = "true" ]; then playwright install chromium; fi && \
if [ "$INCLUDE_FIREFOX" = "true" ]; then playwright install firefox; fi; \
else \
echo "Skipping browser installation"; \
fi

# Copy required files for Python build
COPY pyproject.toml setup.py MANIFEST.in README.md ./
COPY superset-frontend/package.json superset-frontend/
Expand Down Expand Up @@ -214,12 +213,11 @@ FROM python-common AS lean

# Install Python dependencies using docker/pip-install.sh
COPY requirements/base.txt requirements/
RUN --mount=type=cache,target=/root/.cache/uv \
RUN --mount=type=cache,target=${SUPERSET_HOME}/.cache/uv \
/app/docker/pip-install.sh --requires-build-essential -r requirements/base.txt
# Install the superset package
RUN --mount=type=cache,target=/root/.cache/uv \
RUN --mount=type=cache,target=${SUPERSET_HOME}/.cache/uv \
uv pip install .

RUN python -m compileall /app/superset

USER superset
Expand All @@ -238,12 +236,13 @@ RUN /app/docker/apt-install.sh \
# Copy development requirements and install them
COPY requirements/*.txt requirements/
# Install Python dependencies using docker/pip-install.sh
RUN --mount=type=cache,target=/root/.cache/uv \
RUN --mount=type=cache,target=${SUPERSET_HOME}/.cache/uv \
/app/docker/pip-install.sh --requires-build-essential -r requirements/development.txt
# Install the superset package
RUN --mount=type=cache,target=/root/.cache/uv \
RUN --mount=type=cache,target=${SUPERSET_HOME}/.cache/uv \
uv pip install .

RUN uv pip install .[postgres]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

going back to installing postgres client as part of the build process for the dev and ci layer (still not packaging the client on lean)

RUN python -m compileall /app/superset

USER superset
Expand All @@ -252,5 +251,7 @@ USER superset
# CI image...
######################################################################
FROM lean AS ci

USER root
RUN uv pip install .[postgres]
USER superset
CMD ["/app/docker/entrypoints/docker-ci.sh"]
5 changes: 3 additions & 2 deletions docker-compose.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,13 @@ x-superset-volumes: &superset-volumes
- ./docker:/app/docker
- ./superset:/app/superset
- ./superset-frontend:/app/superset-frontend
- superset_home:/app/superset_home
- ./tests:/app/tests
#- superset_home:/app/superset_home

x-common-build: &common-build
context: .
target: ${SUPERSET_BUILD_TARGET:-dev} # can use `dev` (default) or `lean`
#target: ${SUPERSET_BUILD_TARGET:-dev} # can use `dev` (default) or `lean`
target: ci
cache_from:
- apache/superset-cache:3.10-slim-bookworm
args:
Expand Down
8 changes: 6 additions & 2 deletions docker/docker-bootstrap.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,12 @@

set -eo pipefail

# Ensure the temporary directory exists
mkdir -p /tmp/superset_temp

# Make python interactive
if [ "$DEV_MODE" == "true" ]; then
if command -v uv > /dev/null 2>&1; then
if [ "$(whoami)" = "root" ] && command -v uv > /dev/null 2>&1; then
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only doing this when root as superset doesn't have the rights to touch the python packaging.

echo "Reinstalling the app in editable mode"
uv pip install -e .
fi
Expand All @@ -34,7 +37,8 @@ if [ "$CYPRESS_CONFIG" == "true" ]; then
export SUPERSET__SQLALCHEMY_DATABASE_URI=postgresql+psycopg2://superset:superset@db:5432/superset_cypress
PORT=8081
fi
if [[ "$DATABASE_DIALECT" == postgres* ]] ; then
if [[ "$DATABASE_DIALECT" == postgres* ]] && [ "$(whoami)" = "root" ]; then
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to keep this for the use case of other docker-compose files that mount the docker/ folder

# older images may not have the postgres dev requirements installed
echo "Installing postgres requirements"
if command -v uv > /dev/null 2>&1; then
# Use uv in newer images
Expand Down
2 changes: 1 addition & 1 deletion docker/entrypoints/docker-ci.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,4 @@
export SERVER_THREADS_AMOUNT=8
# start up the web server

/usr/bin/run-server.sh
/app/docker/entrypoints/run-server.sh
Loading