sadpandajoe commented on code in PR #44250:
URL: https://github.com/apache/superset/pull/44250#discussion_r4040720548
##########
.github/workflows/docker.yml:
##########
@@ -117,6 +221,11 @@ jobs:
matrix:
build_preset: ${{fromJson(needs.setup_matrix.outputs.matrix_config)}}
fail-fast: false
+ # Apply concurrency after change detection. A docs-only push has no
+ # docker-build job and cannot replace a pending publisher.
+ concurrency:
+ group: ${{ github.workflow }}-${{ github.ref }}-${{ matrix.build_preset
}}
+ cancel-in-progress: false
Review Comment:
This now serializes pull-request Docker jobs with `cancel-in-progress:
false`, so each new commit can wait up to 60 minutes behind builds for an
abandoned merge ref while the old workflow cancelled them. Could this remain
non-cancelling only for publishing pushes and cancel superseded `pull_request`
jobs?
##########
.github/workflows/docker.yml:
##########
@@ -80,23 +75,132 @@ jobs:
# so a regression in supersetbot itself (dropping the py311/py312
# PY_VER pin, or reordering args so our override no longer lands
# last) is caught here too, instead of only surfacing on master.
+ docker_command() {
+ local preset="$1" extra_flags="$2"
+ shift 2
+ # Supersetbot adds its own --cache-to whenever DOCKERHUB_TOKEN is
+ # present. Hide the token from Supersetbot so cache export is
+ # controlled only by docker-build-extra-flags.sh.
+ DOCKERHUB_TOKEN= supersetbot docker \
Review Comment:
The validation assertion blanks `DOCKERHUB_TOKEN` inside its own command
wrapper, so it stays green even if the production guard at line 299 is removed
and release-branch or same-repo validation resumes exporting unreviewed layers
to the shared registry cache. Could the check exercise the production
token-hiding path with a dummy token instead of suppressing Supersetbot's cache
export in the harness?
##########
scripts/docker-build-extra-flags.sh:
##########
@@ -31,18 +31,53 @@
# appending PY_VER here would override supersetbot's pin and silently make
# "py311"/"py312" build the exact same image as "lean". Every other preset
# gets the override so its build lands on the Dockerfile's own supported
-# Python version.
+# Python version. Those presets also point buildx at a matching cache tag so
+# local and CI builds can pull cache layers for the same base image they build.
#
-# Usage: docker-build-extra-flags.sh <build_preset> <image_tag>
+# Usage: docker-build-extra-flags.sh <build_preset> [image_tag] [release]
set -euo pipefail
-BUILD_PRESET="${1:?usage: docker-build-extra-flags.sh <build_preset>
<image_tag>}"
-IMAGE_TAG="${2:?usage: docker-build-extra-flags.sh <build_preset> <image_tag>}"
+BUILD_PRESET="${1:?usage: docker-build-extra-flags.sh <build_preset>
[image_tag]}"
+IMAGE_TAG="${2:-}"
+BUILD_MODE="${3:-ci}"
+DEFAULT_PY_VER="$(sed -n 's/^ARG PY_VER=//p' Dockerfile | head -n 1)"
+if [ -z "$DEFAULT_PY_VER" ]; then
+ echo "Could not determine the default PY_VER from Dockerfile" >&2
+ exit 1
+fi
+
+EXTRA_FLAGS=""
+if [ "$BUILD_MODE" = "ci" ]; then
+ EXTRA_FLAGS="--build-arg INCLUDE_CHROMIUM=false"
+elif [ "$BUILD_MODE" != "release" ]; then
+ echo "Unknown Docker build mode: $BUILD_MODE" >&2
+ exit 1
+fi
+if [ -n "$IMAGE_TAG" ]; then
+ EXTRA_FLAGS="${EXTRA_FLAGS:+$EXTRA_FLAGS }--tag $IMAGE_TAG"
+fi
+case "$BUILD_PRESET" in
+ py311)
+ CACHE_REF="apache/superset-cache:3.11-slim-bookworm"
+ ;;
+ py312)
+ CACHE_REF="apache/superset-cache:3.12-slim-bookworm"
+ ;;
+ *)
+ CACHE_REF="apache/superset-cache:${DEFAULT_PY_VER}"
Review Comment:
This producer now derives the cache tag from `Dockerfile`, but the three
Compose files and two frontend workflows still hard-code `3.11.14-slim-trixie`;
the next routine `PY_VER` bump can move the exporter while every consumer
silently keeps pulling a stale tag, recreating cold builds and the documented
disk-exhaustion path. Could one invariant test bind those five refs to the
Dockerfile default, or derive them from the same source?
--
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]
