You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by as...@apache.org on 2020/06/02 13:54:25 UTC
[airflow] branch master updated: Produce less verbose output when
building docker mount options (#9103)
This is an automated email from the ASF dual-hosted git repository.
ash pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/master by this push:
new 9e0ccde Produce less verbose output when building docker mount options (#9103)
9e0ccde is described below
commit 9e0ccdea2050659958b722c6197b1e319d189c1d
Author: Ash Berlin-Taylor <as...@firemirror.com>
AuthorDate: Tue Jun 2 14:53:43 2020 +0100
Produce less verbose output when building docker mount options (#9103)
The previous method of generating this list had two "problems"/niggles
that this PR solves, when running with VERBOSE=true
- Firstly, LOCAL_MOUNTS was set at the top level, so running with
`set -x` produced 30 extra lines of output.
- Because of the `while read` used, it created 4 or 5 lines _per_ mount,
resulting in a lot verbose output.
Nothing I've changed here is "critical", it's just making it a bit
easier to see with the debug output what is going on, by running fewer
commands.
I have also expanded the BATS test a little bit to check each pair (`-v`
and its following option)
---
scripts/ci/_utils.sh | 89 +++++++++++++++----------------
scripts/ci/pre_commit_local_yml_mounts.sh | 26 ++++-----
tests/bats/test_local_mounts.bats | 27 +++++-----
3 files changed, 64 insertions(+), 78 deletions(-)
diff --git a/scripts/ci/_utils.sh b/scripts/ci/_utils.sh
index 899ea50..e9c3575 100644
--- a/scripts/ci/_utils.sh
+++ b/scripts/ci/_utils.sh
@@ -145,11 +145,7 @@ function initialize_common_environment {
print_info "Mounting necessary host volumes to Docker"
print_info
- EXTRA_DOCKER_FLAGS=()
-
- while IFS= read -r LINE; do
- EXTRA_DOCKER_FLAGS+=( "${LINE}")
- done < <(convert_local_mounts_to_docker_params)
+ read -r -a EXTRA_DOCKER_FLAGS <<< "$(convert_local_mounts_to_docker_params)"
else
print_info
print_info "Skip mounting host volumes to Docker"
@@ -244,53 +240,52 @@ function print_info() {
# By default not the whole airflow sources directory is mounted because there are often
# artifacts created there (for example .egg-info files) that are breaking the capability
# of running different python versions in Breeze. So we only mount what is needed by default.
-LOCAL_MOUNTS="
-.bash_aliases /root/
-.bash_history /root/
-.coveragerc /opt/airflow/
-.dockerignore /opt/airflow/
-.flake8 /opt/airflow/
-.github /opt/airflow/
-.inputrc /root/
-.kube /root/
-.rat-excludes /opt/airflow/
-CHANGELOG.txt /opt/airflow/
-Dockerfile.ci /opt/airflow/
-LICENSE /opt/airflow/
-MANIFEST.in /opt/airflow/
-NOTICE /opt/airflow/
-airflow /opt/airflow/
-backport_packages /opt/airflow/
-common /opt/airflow/
-dags /opt/airflow/
-dev /opt/airflow/
-docs /opt/airflow/
-files /
-dist /
-hooks /opt/airflow/
-logs /root/airflow/
-pylintrc /opt/airflow/
-pytest.ini /opt/airflow/
-requirements /opt/airflow/
-scripts /opt/airflow/
-scripts/ci/in_container/entrypoint_ci.sh /
-setup.cfg /opt/airflow/
-setup.py /opt/airflow/
-tests /opt/airflow/
-tmp /opt/airflow/
-"
+function generate_local_mounts_list {
+ local prefix="$1"
+ LOCAL_MOUNTS=(
+ "$prefix".bash_aliases:/root/.bash_aliases:cached
+ "$prefix".bash_history:/root/.bash_history:cached
+ "$prefix".coveragerc:/opt/airflow/.coveragerc:cached
+ "$prefix".dockerignore:/opt/airflow/.dockerignore:cached
+ "$prefix".flake8:/opt/airflow/.flake8:cached
+ "$prefix".github:/opt/airflow/.github:cached
+ "$prefix".inputrc:/root/.inputrc:cached
+ "$prefix".kube:/root/.kube:cached
+ "$prefix".rat-excludes:/opt/airflow/.rat-excludes:cached
+ "$prefix"CHANGELOG.txt:/opt/airflow/CHANGELOG.txt:cached
+ "$prefix"Dockerfile.ci:/opt/airflow/Dockerfile.ci:cached
+ "$prefix"LICENSE:/opt/airflow/LICENSE:cached
+ "$prefix"MANIFEST.in:/opt/airflow/MANIFEST.in:cached
+ "$prefix"NOTICE:/opt/airflow/NOTICE:cached
+ "$prefix"airflow:/opt/airflow/airflow:cached
+ "$prefix"backport_packages:/opt/airflow/backport_packages:cached
+ "$prefix"common:/opt/airflow/common:cached
+ "$prefix"dags:/opt/airflow/dags:cached
+ "$prefix"dev:/opt/airflow/dev:cached
+ "$prefix"docs:/opt/airflow/docs:cached
+ "$prefix"files:/files:cached
+ "$prefix"dist:/dist:cached
+ "$prefix"hooks:/opt/airflow/hooks:cached
+ "$prefix"logs:/root/airflow/logs:cached
+ "$prefix"pylintrc:/opt/airflow/pylintrc:cached
+ "$prefix"pytest.ini:/opt/airflow/pytest.ini:cached
+ "$prefix"requirements:/opt/airflow/requirements:cached
+ "$prefix"scripts:/opt/airflow/scripts:cached
+ "$prefix"scripts/ci/in_container/entrypoint_ci.sh:/entrypoint_ci.sh:cached
+ "$prefix"setup.cfg:/opt/airflow/setup.cfg:cached
+ "$prefix"setup.py:/opt/airflow/setup.py:cached
+ "$prefix"tests:/opt/airflow/tests:cached
+ "$prefix"tmp:/opt/airflow/tmp:cached
+ )
+}
# Converts the local mounts that we defined above to the right set of -v
# volume mappings in docker-compose file. This is needed so that we only
# maintain the volumes in one place (above)
function convert_local_mounts_to_docker_params() {
- echo "${LOCAL_MOUNTS}" |sed '/^$/d' | awk -v AIRFLOW_SOURCES="${AIRFLOW_SOURCES}" \
- '
- function basename(file) {
- sub(".*/", "", file)
- return file
- }
- { print "-v"; print AIRFLOW_SOURCES "/" $1 ":" $2 basename($1) ":cached" }'
+ generate_local_mounts_list "${AIRFLOW_SOURCES}/"
+ # Bash can't "return" arrays, so we need to quote any special characters
+ printf -- '-v %q ' "${LOCAL_MOUNTS[@]}"
}
# Fixes a file that is expected to be a file. If - for whatever reason - the local file is not created
diff --git a/scripts/ci/pre_commit_local_yml_mounts.sh b/scripts/ci/pre_commit_local_yml_mounts.sh
index a614184..d77a528 100755
--- a/scripts/ci/pre_commit_local_yml_mounts.sh
+++ b/scripts/ci/pre_commit_local_yml_mounts.sh
@@ -23,29 +23,21 @@ MY_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
# shellcheck source=scripts/ci/_script_init.sh
. "$( dirname "${BASH_SOURCE[0]}" )/_script_init.sh"
-TMP_FILE=$(mktemp)
TMP_OUTPUT=$(mktemp)
+# Remove temp file if it's hanging around
+trap 'rm -rf -- "${TMP_OUTPUT}" 2>/dev/null' EXIT
+
LOCAL_YML_FILE="${MY_DIR}/docker-compose/local.yml"
LEAD=' # START automatically generated volumes from LOCAL_MOUNTS in _utils.sh'
TAIL=' # END automatically generated volumes from LOCAL_MOUNTS in _utils.sh'
-echo "${LOCAL_MOUNTS}" |sed '/^$/d' | \
- awk '
- function basename(file) {
- sub(".*/", "", file)
- return file
- }
- { print " - ../../../" $1 ":" $2 basename($1) ":cached"}
- ' > "${TMP_FILE}"
-
-
-BEGIN_GEN=$(grep -n "${LEAD}" <"${LOCAL_YML_FILE}" | sed 's/\(.*\):.*/\1/g')
-END_GEN=$(grep -n "${TAIL}" <"${LOCAL_YML_FILE}" | sed 's/\(.*\):.*/\1/g')
-cat <(head -n "${BEGIN_GEN}" "${LOCAL_YML_FILE}") \
- "${TMP_FILE}" \
- <(tail -n +"${END_GEN}" "${LOCAL_YML_FILE}") \
- >"${TMP_OUTPUT}"
+generate_local_mounts_list " - ../../../"
+
+sed -ne "0,/$LEAD/ p" "${LOCAL_YML_FILE}" > "${TMP_OUTPUT}"
+
+printf '%s\n' "${LOCAL_MOUNTS[@]}" >> "${TMP_OUTPUT}"
+sed -ne "/$TAIL/,\$ p" "${LOCAL_YML_FILE}" >> "${TMP_OUTPUT}"
mv "${TMP_OUTPUT}" "${LOCAL_YML_FILE}"
diff --git a/tests/bats/test_local_mounts.bats b/tests/bats/test_local_mounts.bats
index 89ae992..97cf984 100644
--- a/tests/bats/test_local_mounts.bats
+++ b/tests/bats/test_local_mounts.bats
@@ -23,19 +23,18 @@
initialize_common_environment
- run convert_local_mounts_to_docker_params
+ read -r -a RES <<< "$(convert_local_mounts_to_docker_params)"
- RES="${output}"
- COUNT_LINES=$(grep -c '' <(echo "${RES}"))
- COUNT_MINUS_V_LINES=$(grep -c '^-v$' <(echo "${RES}"))
- if (( COUNT_LINES != 2*COUNT_MINUS_V_LINES )); then
- echo "The output produced by the convert_local_mounts_to_docker_params function is wrong"
- echo "There are ${COUNT_LINES} lines in total and ${COUNT_MINUS_V_LINES} lines with -v"
- echo "They seconf number should be exactly 1/2 of the first."
- echo
- echo "The output below should contain interleaving -v / volume lines"
- echo
- echo "${RES}"
- exit 1
- fi
+ [[ ${#RES[@]} -gt 0 ]] # Array should be non-zero length
+ [[ $((${#RES[@]} % 2)) == 0 ]] # Array should be even length
+
+ for i in "${!RES[@]}"; do
+ if [[ $((i % 2)) == 0 ]]; then
+ # Every other value should be `-v`
+ [[ ${RES[$i]} == "-v" ]]
+ else
+ # And the options should be of form <src>:<dest>:cached
+ [[ ${RES[$i]} = *:*:cached ]]
+ fi
+ done
}