kgyrtkirk commented on code in PR #17592:
URL: https://github.com/apache/druid/pull/17592#discussion_r1903989935


##########
integration-tests/docker/Dockerfile:
##########
@@ -27,15 +27,19 @@ ARG ZK_VERSION
 ARG APACHE_ARCHIVE_MIRROR_HOST=https://archive.apache.org
 ARG SETUP_RETRIES=3
 # Retry mechanism for running the setup script up to limit of 3 times.
-RUN set -e; \
-    for i in $(seq 1 $SETUP_RETRIES); do \
-        echo "Attempt $i to run the setup script..."; \
+RUN i=0; \
+    while [ $i -lt $SETUP_RETRIES ]; do \
         APACHE_ARCHIVE_MIRROR_HOST=${APACHE_ARCHIVE_MIRROR_HOST} 
/root/base-setup.sh && break || { \
+            i=$(($i + 1)); \
             echo "Set up script attempt $i/$SETUP_RETRIES failed."; \
             sleep 2; \
         }; \
     done; \
-    rm -f /root/base-setup.sh
+    rm -f /root/base-setup.sh; \
+    if [ "$i" -eq "$SETUP_RETRIES" ]; then \
+        exit 1; \

Review Comment:
   this seems like a fix-of-the-fix already #17543
   
   I think this retry logic is completely misplaced; it seems to me that the 
original problem was that `wget` times out?
   I think running all steps of the script will retry it even if other issues 
happen ...
   
   I wonder why not specify some retry conditions for `wget` instead ?
   
https://unix.stackexchange.com/questions/227665/wget-abort-retrying-after-failure-or-timeout
   



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