[GitHub] [airflow] potiuk commented on a change in pull request #10651: Implement better shell conventions for breeze

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

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



##########
File path: scripts/in_container/_in_container_utils.sh
##########
@@ -16,6 +16,27 @@
 # specific language governing permissions and limitations
 # under the License.
 
+#######################################################################################################
+#
+# Adds trap to the traps already set.
+#
+# Arguments:
+#      trap to set
+#      .... list of signals to handle
+#######################################################################################################
+function add_trap() {
+    trap="${1}"
+    shift
+    for signal in "${@}"
+    do
+        # adding trap to exiting trap
+        local handlers
+        handlers="$( trap -p "${signal}" | cut -f2 -d \' )"
+        # shellcheck disable=SC2064
+        trap "${handlers}${handlers:+;}${trap}" "${signal}"
+    done
+}
+

Review comment:
       Not really. The problem is that there are no common parts between the "in_container" and "out_container" stuff. We are even excluding the non-in-container scripts when we prepare the images, to make sure that what we run outside of the container is not run inside (by default). I think it's important to keep that separation. This is a really small piece of code that does not really require maintenance - kind of a "util" class (and a nice one to have at that - since we already use traps in several places and traps - unfortunately - in shell scripts are simply replacing previous traps rather than add to them.




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