gemini-code-assist[bot] commented on code in PR #36547:
URL: https://github.com/apache/beam/pull/36547#discussion_r2437126509
##########
sdks/python/container/run_validatescontainer.sh:
##########
@@ -99,15 +99,29 @@ fi
function cleanup_container {
# Delete the container locally and remotely
docker rmi $CONTAINER:$TAG || echo "Built container image was not removed.
Possibly, it was not not saved locally."
- for image in $(docker images --format '{{.Repository}}:{{.Tag}}' | grep
$PREBUILD_SDK_CONTAINER_REGISTRY_PATH)
+
+ for image in $(docker images --format '{{.Repository}}:{{.Tag}}' | grep
$PREBUILD_SDK_CONTAINER_REGISTRY_PATH | grep -E
"(beam_python_prebuilt_sdk|$TAG)")
do
echo "Deleting Docker image: $image"
docker rmi $image || echo "Failed to remove prebuilt sdk container image"
image_tag="${image##*:}"
- digest=$(gcloud container images list-tags
$PREBUILD_SDK_CONTAINER_REGISTRY_PATH/beam_python_prebuilt_sdk
--filter="tags=$image_tag" --format="get(digest)")
- echo "Deleting from GCloud an image with digest: $digest"
- gcloud container images delete
$PREBUILD_SDK_CONTAINER_REGISTRY_PATH/beam_python_prebuilt_sdk@$digest
--force-delete-tags --quiet || echo "Failed to remove prebuilt sdk container
image"
+
+ if [[ -n "$image_tag" && ("$image_tag" == *"$TAG"* || "$image_tag" ==
*"beam_python_prebuilt_sdk"*) ]]; then
+ digest=$(gcloud container images list-tags
$PREBUILD_SDK_CONTAINER_REGISTRY_PATH/beam_python_prebuilt_sdk
--filter="tags=$image_tag" --format="get(digest)")
+
+ echo "Looking for digest for tag '$image_tag', found: '$digest'"
+
+ if [[ -n "$digest" && "$digest" =~ ^sha256:[a-f0-9]{64}$ ]]; then
+ echo "Deleting from GCloud an image with digest: $digest"
+ gcloud container images delete
$PREBUILD_SDK_CONTAINER_REGISTRY_PATH/beam_python_prebuilt_sdk@$digest
--force-delete-tags --quiet || echo "Failed to remove prebuilt sdk container
image"
+ else
+ echo "Skipping deletion of image with invalid or empty digest:
'$digest'"
+ fi
+ else
+ echo "Skipping deletion of image from other runs: $image"
+ fi
Review Comment:
The `if` condition on line 109 appears to be redundant. The `for` loop on
line 103 already filters the images using `grep -E
"(beam_python_prebuilt_sdk|$TAG)"`. This `grep` is applied to the `repo:tag`
string. Since the repository path (`$PREBUILD_SDK_CONTAINER_REGISTRY_PATH`)
does not contain `beam_python_prebuilt_sdk` or `$TAG`, the `grep` effectively
filters by the image tag.
Therefore, the `if` condition `[[ -n "$image_tag" && ("$image_tag" ==
*"$TAG"* || "$image_tag" == *"beam_python_prebuilt_sdk"*) ]]` will always be
true for images processed in the loop, and the `else` block on line 120 will
never be reached.
I suggest removing this outer `if/else` structure to simplify the code and
avoid redundancy.
```suggestion
digest=$(gcloud container images list-tags
$PREBUILD_SDK_CONTAINER_REGISTRY_PATH/beam_python_prebuilt_sdk
--filter="tags=$image_tag" --format="get(digest)")
echo "Looking for digest for tag '$image_tag', found: '$digest'"
if [[ -n "$digest" && "$digest" =~ ^sha256:[a-f0-9]{64}$ ]]; then
echo "Deleting from GCloud an image with digest: $digest"
gcloud container images delete
$PREBUILD_SDK_CONTAINER_REGISTRY_PATH/beam_python_prebuilt_sdk@$digest
--force-delete-tags --quiet || echo "Failed to remove prebuilt sdk container
image"
else
echo "Skipping deletion of image with invalid or empty digest:
'$digest'"
fi
```
--
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]
