rusackas opened a new pull request, #42509:
URL: https://github.com/apache/superset/pull/42509

   ### SUMMARY
   While looking into whether `docker/bake-action` would help the docker build 
matrix (`dev`/`lean`/`websocket`/`dockerize`/`py311`/`py312`), I noticed 
something unrelated to bake: `docker.yml` appends `--build-arg 
PY_VER=3.11.14-slim-trixie` via `--extra-flags` for every preset in the matrix, 
so it can pin `dev`/`lean` to the Dockerfile's actual supported Python version 
(supersetbot's own default for those two is stale). But `docker/buildx` keeps 
the last value for a repeated `--build-arg` key, and that override was landing 
after supersetbot's own `--build-arg PY_VER` for `py311`/`py312` too, silently 
clobbering the one thing that's supposed to make those two presets different 
from `lean`. `lean`, `py311`, and `py312` have been building the same image.
   
   This excludes `py311`/`py312` from that override so their own `PY_VER` pin 
actually survives.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   N/A, CI-only change.
   
   ### TESTING INSTRUCTIONS
   `zizmor` and `pre-commit` both pass clean. The real test is the docker-build 
matrix on this PR itself: `py311` and `py312` should now show a different 
`base=` label than `lean` in the "Print docker stats" step (`docker history 
$IMAGE_TAG`), where before all three matched.
   
   ### ADDITIONAL INFORMATION
   While in here, a few other things came up that I'm treating as separate 
follow-ups rather than folding into this PR:
   
   - `docker/bake-action` itself: `dev`/`lean`/`py311`/`py312` all share one 
Dockerfile and its ancestor stages (notably the Node/frontend build stage, 
which doesn't even depend on `PY_VER`), so building them via one `docker buildx 
bake` invocation instead of 4 separate matrix legs would cut real redundant 
work. `websocket` and `dockerize` are unrelated Dockerfiles with no shared 
`FROM` ancestry, so they wouldn't gain anything from being folded in. This is a 
bigger lift: it means teaching supersetbot to emit a bake file, and it trades 
runner-level parallelism (today's legs run concurrently on separate runners) 
for one job doing the shared work once, so it's a compute/cost win more than a 
guaranteed wall-clock win unless paired with a beefier runner. I'll open that 
as its own PR against supersetbot.
   - There's already a registry-backed buildx cache 
(`--cache-from`/`--cache-to` against `apache/superset-cache:<pyVer>`) per 
preset, so this isn't a from-scratch-every-time situation, just one where 
shared stages get pulled/pushed redundantly across legs instead of built once.
   - Unrelated oddity I didn't touch: the build step references `$EVENT`, 
`$RELEASE`, and `$FORCE_LATEST`, none of which are set anywhere in this 
workflow. supersetbot falls back to deriving context/ref from 
`GITHUB_EVENT_NAME`/`GITHUB_REF` when they're empty, so this looks harmless 
as-is, but the dead variables are worth a cleanup pass sometime.
   
   - [ ] Has associated issue:
   - [ ] Required feature flags:
   - [ ] Changes UI
   - [ ] Includes DB Migration
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   


-- 
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]

Reply via email to