sadpandajoe commented on code in PR #44248:
URL: https://github.com/apache/superset/pull/44248#discussion_r4011928770
##########
.github/workflows/scheduled-docker-image-refresh.yml:
##########
@@ -151,20 +151,64 @@ jobs:
LATEST_RELEASE: ${{ needs.config.outputs.latest-release }}
FORCE_LATEST_FLAG: ${{ needs.config.outputs.force-latest == '1' &&
'--force-latest' || '' }}
run: |
+ set -euo pipefail
+
# Reuses the same supersetbot invocation as the release
# publisher (`tag-release.yml`), so the resulting tags are
# identical to what a manual release dispatch would produce —
# just with a freshly-pulled base image layer underneath.
# `--force-latest` is only passed when the config job confirmed the
# fetched release is the newest one (see FORCE_LATEST_FLAG above).
+ #
+ # supersetbot's preset -> Dockerfile-target mapping tracks current
+ # master, but this job builds an arbitrary *historical* release ref
+ # (checked out above). A preset introduced after that release won't
+ # have a matching stage in its Dockerfile. That's exactly what broke
+ # here (#44220): `superset` was added as both a new Dockerfile stage
+ # and the preset backing the plain tags (`latest`, `<version>`,
+ # per-SHA) by #44100. Releases cut before it, like 6.1.0, still have
+ # their plain tags built from the pre-existing `lean` stage — that
+ # was the plain tag's source before #44100 split it out (see
+ # docs/admin_docs/installation/docker-builds.mdx as of that commit)
+ # — so `lean` is the correct substitute, not a guess.
+ #
+ # Detect the mismatch by asking supersetbot what it would actually
+ # run (--dry-run) and checking the target stage it picked against
+ # the Dockerfile actually checked out for this release.
+ EXTRA_FLAGS=""
+ DRY_RUN_CMD="$(supersetbot docker \
+ --preset "$BUILD_PRESET" \
+ --context release \
+ --context-ref "$LATEST_RELEASE" \
+ $FORCE_LATEST_FLAG \
+ --platform linux/amd64 \
+ --dry-run)"
+ # `|| true`: under `set -e`/`pipefail`, a `--dry-run` command with no
+ # `--target` in its output (grep matches nothing) would otherwise
+ # abort the whole step right here with no diagnostic at all, rather
+ # than leaving TARGET_STAGE empty so the check below can skip
+ # cleanly.
+ TARGET_STAGE="$(grep -oE -- '--target [A-Za-z0-9_.-]+'
<<<"$DRY_RUN_CMD" | tail -1 | cut -d' ' -f2 || true)"
+
+ if [ -n "$TARGET_STAGE" ] && ! grep -qiE "^[[:space:]]*FROM .* AS
${TARGET_STAGE}\$" Dockerfile; then
Review Comment:
This compatibility branch is the only thing keeping pre-#44100 releases from
retrying the missing `superset` target, but the committed tests never execute
it; they only inspect action checkout, notifier labels, and change detection,
so a parser or flag-order regression would restore the weekly failure while CI
stays green. Could we factor this into a small helper and test the lean-only
`superset` fallback, no-target pass-through, and unknown-mismatch refusal at
the assembled-command boundary?
--
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]
