hudi-agent commented on code in PR #19489:
URL: https://github.com/apache/hudi/pull/19489#discussion_r3717718380


##########
docker/README.md:
##########
@@ -125,10 +127,28 @@ After new images are built, you can run the following 
script to bring up docker
 ./setup_demo.sh dev
 ```
 
-## Upload Updated Image to Repository on Docker Hub
+## Publishing Images to Docker Hub
+
+Publishing is a release action for Hudi maintainers, not part of the normal 
build or demo flow. It overwrites the
+images that every `setup_demo.sh` user pulls, so it needs write access to the 
`apachehudi/...` Docker Hub
+repositories and a prior `docker login`. If you are contributing a Dockerfile 
change, build without `--publish true` and
+let a maintainer publish.
 
-Once you have built the updated image locally, you can push the corresponding 
this repository of the image to the Docker
-Hud registry designated by its name or tag:
+`build_and_publish_docker_images.sh` publishes the whole image set it just 
built when given `--publish true`:
+
+```shell
+# Build the image set and publish it
+./build_and_publish_docker_images.sh --publish true
+```
+
+Each image is pushed under both the `latest` tag and the version tag, the 
latter taken from the root `pom.xml`
+unless `VERSION_TAG` overrides it. The script reports a per-tag summary and 
exits non-zero if any push fails, so a

Review Comment:
   🤖 This section states the script "exits non-zero if any push fails, so a 
partial publish is not mistaken for a successful one," but the current 
`push_images()` in `build_and_publish_docker_images.sh` doesn't actually do 
that. Its failure branch only prints `Some pushes failed. Review logs above.` 
with no `exit 1`/`return 1`, and since each `docker push` runs inside an `if` 
(exempt from `set -e`) and the final command is an `echo`, the script exits 0 
even when some images (or all, if inspect finds them missing) failed to push. 
Because publishing overwrites the shared `apachehudi/...` repos, a maintainer 
gating a release on the exit code would read a partial publish as success and 
leave the registry mixed (some images at the new tag, some stale). Could we 
either add an `exit 1` when `FAILURE_COUNT > 0` in the script, or soften this 
sentence to match the actual behavior? The per-tag summary claim is accurate; 
it's just the exit-code guarantee that isn't.
   
   <sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag 
quality.</i></sub>



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

Reply via email to