sadpandajoe commented on code in PR #44250:
URL: https://github.com/apache/superset/pull/44250#discussion_r4011551718
##########
UPDATING.md:
##########
@@ -47,6 +47,20 @@ but under `-lean` tags: `latest-lean`, `master-lean`,
`5.0.0-lean`, `<sha>-lean`
drivers most installations need are now present out of the box.
- The `-dev` images (`latest-dev`, `master-dev`, …) are unchanged.
+### Docker image publishing now excludes standalone `websocket` and
`dockerize` images
+
+The Apache Superset Docker Hub repository no longer publishes standalone
+`apache/superset:*websocket` or `apache/superset:*dockerize` image tags. The
+realtime WebSocket server is bundled in the `superset`, `lean`, and `dev`
images
+and can be launched with `/app/docker/entrypoints/run-websocket.sh`. Helm init
+containers use the main Superset image for dependency checks.
+
+Release branch pushes no longer publish Docker Hub images or cache layers; they
+validate the Docker build locally. Official release tags (`X.Y.Z`, `latest`,
and
+their preset variants) are published only by the release workflow after release
+manager sign-off. The scheduled release-image refresh workflow was removed, so
Review Comment:
Removing the scheduled publisher leaves
`tests/unit_tests/scheduled_docker_image_refresh_test.py` in this head, and its
two `WORKFLOW_PATH.read_text()` tests now fail with `FileNotFoundError`; the
current `unit-tests-required` check is red on exactly those failures. Should
this delete that test and the stale `scripts/change_detector.py` path matcher
with the workflow?
##########
.github/workflows/docker.yml:
##########
@@ -80,23 +79,88 @@ 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 \
+ --preset "$preset" \
+ --platform linux/amd64 \
+ "$@" \
+ --extra-flags "$extra_flags" \
+ --dry-run
+ }
assert_effective_py_ver() {
local preset="$1" expected="$2" extra_flags command actual
extra_flags="$(scripts/docker-build-extra-flags.sh "$preset"
dummy-tag)"
- command="$(supersetbot docker --preset "$preset" --platform
linux/amd64 --extra-flags "$extra_flags" --dry-run)"
+ command="$(docker_command "$preset" "$extra_flags")"
# docker buildx keeps the LAST value of a repeated --build-arg key.
actual="$(grep -oE -- '--build-arg PY_VER=[^[:space:]]+'
<<<"$command" | tail -1)"
if [ "$actual" != "--build-arg PY_VER=$expected" ]; then
echo "::error::preset '$preset' expected effective --build-arg
PY_VER=$expected, got: ${actual:-<none>} (full command: $command)"
exit 1
fi
}
- for preset in superset dev lean websocket dockerize; do
+ assert_cache_ref() {
+ local preset="$1" expected="$2" extra_flags command
+ extra_flags="$(scripts/docker-build-extra-flags.sh "$preset"
dummy-tag)"
+ command="$(docker_command "$preset" "$extra_flags")"
+ if ! grep -Fq --
"--cache-from=type=registry,ref=apache/superset-cache:$expected" <<<"$command";
then
+ echo "::error::preset '$preset' expected cache-from
apache/superset-cache:$expected (full command: $command)"
+ exit 1
+ fi
+ }
+ assert_master_tag() {
+ local preset="$1" expected="$2" extra_flags command
+ extra_flags="$(scripts/docker-build-extra-flags.sh "$preset"
dummy-tag)"
+ command="$(docker_command "$preset" "$extra_flags" --context push
--context-ref master)"
Review Comment:
This assertion hard-codes `push`/`master` instead of exercising the
`BUILD_CONTEXT`/`BUILD_CONTEXT_REF` wiring that this PR fixes. A refactor can
pass an empty or wrong ref again, leave this job green, and freeze every
`master*` tag after merge; could the event/ref decision be shared with the
build step and tested for master, release-branch, and pull-request inputs?
##########
.github/workflows/docker.yml:
##########
@@ -162,15 +226,27 @@ jobs:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
BUILD_PRESET: ${{ matrix.build_preset }}
run: |
- # Single platform builds in pull_request context to speed things up
- if [ "$GITHUB_EVENT_NAME" = "push" ]; then
+ set -euo pipefail
+ BUILD_CONTEXT="$GITHUB_EVENT_NAME"
+ BUILD_CONTEXT_REF=""
+ PUBLISH_DOCKER_CACHE=""
+
+ if [ "$GITHUB_EVENT_NAME" = "push" ] && [ "$GITHUB_REF" =
"refs/heads/master" ]; then
PLATFORM_ARG="--platform linux/arm64 --platform linux/amd64"
- # can only --load images in single-platform builds
PUSH_OR_LOAD="--push"
Review Comment:
Restoring mutable `master*` tags without serializing master publish jobs
lets an older, slower run finish after a newer one and move those tags
backward. Could the master path use a stable concurrency group or recheck that
its SHA is still the tip before pushing mutable tags?
##########
.github/workflows/tag-release.yml:
##########
@@ -55,8 +55,7 @@ jobs:
contents: write
strategy:
matrix:
- build_preset:
- ["superset", "dev", "lean", "websocket", "dockerize", "py311",
"py312"]
+ build_preset: ["superset", "dev", "lean", "py311", "py312"]
Review Comment:
This makes `tag-release.yml` the sole official publisher, but these
remaining presets still call Supersetbot without the new extra-flags helper.
Supersetbot 0.5.45 emits `PY_VER=3.10-slim-bookworm` for
`superset`/`dev`/`lean`, so an official release can fail or publish on the
interpreter that `pyproject.toml` declares unsupported while the new
master-only command check stays green; should the release job use the same
helper and assembled-command assertion?
--
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]
