bito-code-review[bot] commented on code in PR #43705: URL: https://github.com/apache/superset/pull/43705#discussion_r3892572826
########## Containerfile.podman: ########## @@ -0,0 +1,293 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +###################################################################### +# Node stage to deal with static asset construction +###################################################################### +ARG PY_VER=3.11.14-slim-trixie + + +# Include translations in the final build +ARG BUILD_TRANSLATIONS="false" + +###################################################################### +# superset-node-ci used as a base for building frontend assets and CI +###################################################################### +FROM node:24-trixie-slim AS superset-node-ci +RUN printf '%s\n' 'Acquire::ForceIPv4 "true";' > /etc/apt/apt.conf.d/99force-ipv4 +ARG BUILD_TRANSLATIONS +ENV BUILD_TRANSLATIONS=${BUILD_TRANSLATIONS} +ARG DEV_MODE="false" # Skip frontend build in dev mode +ENV DEV_MODE=${DEV_MODE} + +COPY docker/ /app/docker/ +# Arguments for build configuration +ARG NPM_BUILD_CMD="build" + +# Install system dependencies required for node-gyp +RUN /app/docker/apt-install.sh build-essential python3 zstd + +# Define environment variables for frontend build +ENV BUILD_CMD=${NPM_BUILD_CMD} \ + PUPPETEER_SKIP_CHROMIUM_DOWNLOAD=true + +# Run the frontend memory monitoring script +RUN /app/docker/frontend-mem-nag.sh + +WORKDIR /app/superset-frontend + +# Create necessary folders to avoid errors in subsequent steps +RUN mkdir -p /app/superset/static/assets \ + /app/superset/translations + +# Harden `npm ci` against transient npm-registry network blips (e.g. ECONNRESET), +# which otherwise fail the entire multi-platform image build with no retry. +ENV npm_config_fetch_retries=5 \ + npm_config_fetch_retry_mintimeout=20000 \ + npm_config_fetch_retry_maxtimeout=120000 \ + npm_config_fetch_timeout=600000 + +# Mount package files and install dependencies if not in dev mode +# NOTE: we mount packages and plugins as they are referenced in package.json as workspaces +# ideally we'd COPY only their package.json. Here npm ci will be cached as long +# as the full content of these folders don't change, yielding a decent cache reuse rate. +# Note that it's not possible to selectively COPY or mount using blobs. +RUN --mount=type=bind,source=./superset-frontend/package.json,target=./package.json \ + --mount=type=bind,source=./superset-frontend/package-lock.json,target=./package-lock.json \ + --mount=type=cache,target=/root/.cache \ + --mount=type=cache,target=/root/.npm \ + if [ "${DEV_MODE}" = "false" ]; then \ + npm ci; \ + else \ + echo "Skipping 'npm ci' in dev mode"; \ + fi + +# Runs the webpack build process +COPY superset-frontend /app/superset-frontend + +###################################################################### +# superset-node is used for compiling frontend assets +###################################################################### +FROM superset-node-ci AS superset-node + +# Build the frontend if not in dev mode +RUN --mount=type=cache,target=/root/.npm \ + if [ "${DEV_MODE}" = "false" ]; then \ + echo "Running 'npm run ${BUILD_CMD}'"; \ + npm run ${BUILD_CMD}; \ + else \ + echo "Skipping 'npm run ${BUILD_CMD}' in dev mode"; \ + fi; + +# Copy translation files +COPY superset/translations /app/superset/translations + +# Build translations if enabled, then cleanup localization files +RUN if [ "${BUILD_TRANSLATIONS}" = "true" ]; then \ + npm run build-translation; \ + fi; \ + rm -rf /app/superset/translations/*/*/*.[po,mo]; + + +###################################################################### +# Base python layer +###################################################################### +FROM python:${PY_VER} AS python-base +RUN printf '%s\n' 'Acquire::ForceIPv4 "true";' > /etc/apt/apt.conf.d/99force-ipv4 + +ARG SUPERSET_HOME="/app/superset_home" +ENV SUPERSET_HOME=${SUPERSET_HOME} + +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/ + +RUN pip install --no-cache-dir --upgrade uv + +# Using uv as it's faster/simpler than pip +RUN uv venv /app/.venv +ENV PATH="/app/.venv/bin:${PATH}" + +###################################################################### +# Python translation compiler layer +###################################################################### +FROM python-base AS python-translation-compiler + +ARG BUILD_TRANSLATIONS +ENV BUILD_TRANSLATIONS=${BUILD_TRANSLATIONS} + +# Install Python dependencies using docker/pip-install.sh +COPY requirements/translations.txt requirements/ +RUN --mount=type=cache,target=/root/.cache/uv \ + . /app/.venv/bin/activate && /app/docker/pip-install.sh --requires-build-essential -r requirements/translations.txt + +COPY superset/translations/ /app/translations_mo/ +RUN if [ "${BUILD_TRANSLATIONS}" = "true" ]; then \ + pybabel compile --use-fuzzy -d /app/translations_mo || true; \ + fi; \ + rm -f /app/translations_mo/*/*/*.[po,json] + +###################################################################### +# Python APP common layer +###################################################################### +FROM python-base AS python-common + +ENV SUPERSET_HOME="/app/superset_home" \ + 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 +RUN mkdir -p \ + ${PYTHONPATH} \ + superset/static \ + requirements \ + superset-frontend \ + apache_superset.egg-info \ + requirements \ + && touch superset/static/version_info.json + +# Install Playwright and optionally setup headless browsers +ENV PLAYWRIGHT_BROWSERS_PATH=/usr/local/share/playwright-browsers + +ARG INCLUDE_CHROMIUM="false" +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/ +COPY scripts/check-env.py scripts/ + +# keeping for backward compatibility +COPY --chmod=755 ./docker/entrypoints/run-server.sh /usr/bin/ + +# Some debian libs +RUN /app/docker/apt-install.sh \ + curl \ + libsasl2-dev \ + libsasl2-modules-gssapi-mit \ + libpq-dev \ + libecpg-dev \ + libldap2-dev + +# Create data directory for DuckDB examples database +# The database file will be created at runtime when examples are loaded from Parquet files +RUN mkdir -p /app/data && chown -R superset:superset /app/data + +# Copy compiled things from previous stages +COPY --from=superset-node /app/superset/static/assets superset/static/assets +# Copy service.worker.js optionall as it doesn't exist when DEV_MODE=true + +# TODO, when the next version comes out, use --exclude superset/translations +COPY superset superset Review Comment: <div> <div id="suggestion"> <div id="issue"><b>Missing service-worker.js COPY</b></div> <div id="fix"> The comment on line 212 describes copying `service-worker.js`, but the actual COPY is missing compared to the upstream `Dockerfile` (line 213). `webpack.config.js` emits `service-worker.js` to `superset/static/`, and `spa.html` registers `/static/service-worker.js` at runtime (served with a `Service-Worker-Allowed` header in `initialization/__init__.py`). Without this COPY the final image 404s on PWA registration. Restore it with the `[s]` bracket trick. </div> <details> <summary> <b>Code suggestion</b> </summary> <blockquote>Check the AI-generated fix before applying</blockquote> <div id="code"> ````suggestion # Copy service.worker.js optionall as it doesn't exist when DEV_MODE=true COPY --from=superset-node /app/superset/static/service-worker.j[s] superset/static/service-worker.js # TODO, when the next version comes out, use --exclude superset/translations COPY superset superset ```` </div> </details> </div> <small><i>Code Review Run #bb3a22</i></small> </div> --- Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>) - [ ] Yes, avoid them -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
