[GitHub] [airflow] potiuk commented on a change in pull request #11535: Fix random kills during pre-commit image building

[GitBox <gi...@apache.org>]

potiuk commented on a change in pull request #11535:
URL: https://github.com/apache/airflow/pull/11535#discussion_r507200556



##########
File path: scripts/ci/libraries/_build_images.sh
##########
@@ -519,8 +519,8 @@ function build_images::build_ci_image() {
         " >"${DETECTED_TERMINAL}"
         spinner::spin "${OUTPUT_LOG}" &
         SPIN_PID=$!
-        # shellcheck disable=SC2064
-        traps::add_trap "kill ${SPIN_PID} || true" INT TERM HUP EXIT
+        # shellcheck disable=SC2064,SC2016
+        traps::add_trap '$(kill '${SPIN_PID}' || true)' EXIT HUP INT TERM

Review comment:
       Nope. It is entirely correct. Look where the scope of ' starts/ends.
   
   I want the SPIN_PID to be resolved at the time when the add_trap command line parameter gets evaluated. The solution you proposed will make it evaluated at the moment trap will be executed.
   
   ```
   SPIN_PID=10
   traps::add_trap '$(kill '${SPIN_PID}' || true)' EXIT HUP INT TERM
   ```
   will be evaluated to 
   
   ```
   traps::add_trap '$(kill 10 || true)' EXIT HUP INT TERM
   ```
   
   Simply the ${SPIN_PID} is between the two '-delimite striings  
   
   And this is exactly what I want. 




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org