[AIRFLOW-5369] Adds interactivity to pre-commits (apache#5976)

This commit adds full interactivity to pre-commits. Whenever you run pre-commit
and it detects that the image should be rebuild, an interactive question will
pop up instead of failing the build and asking to rebuild with REBUILD=yes

This is much nicer from the user perspective. You can choose whether to:
1) Rebuild the image (which will take some time)
2) Not rebuild the image (this will use the old image with hope it's OK)
3) Quit.

Answer to that question is carried across all images needed to rebuild.
There is the special "build" pre-commit hook that takes care about that.

Note that this interactive question cannot be asked if you run only
single pre-commit hook with Dockerfile because it can run multiple processes
and you can start building in parallel. This is not desired so instead we fail
such builds.

Author: potiuk

.pre-commit-config.yaml

@@ -24,13 +24,13 @@ repos:
     hooks:
       - id: build
         name: Check if image build is needed
-        entry: ./scripts/ci/ci_build.sh
+        entry: ./scripts/ci/local_ci_build.sh
         language: system
         always_run: true
         pass_filenames: false
       - id: check-apache-license
         name: Check if licences are OK for Apache
-        entry: ./scripts/ci/ci_check_license.sh
+        entry: "./scripts/ci/pre_commit_check_license.sh"
         language: system
         files: ^.*LICENSE.*$|^.*LICENCE.*$
         pass_filenames: false
@@ -177,32 +177,37 @@ repos:
         language: docker_image
         entry: koalaman/shellcheck:stable -x -a
         types: [shell]
-        files: ^breeze$|^breeze-complete$|\.sh$
+        files: ^breeze$|^breeze-complete$|\.sh$|^hooks/build$|^hooks/push$
         exclude: ^airflow/_vendor/.*$
       - id: lint-dockerfile
         name: Lint dockerfile
         language: system
-        entry: ./scripts/ci/ci_lint_dockerfile.sh
+        entry: "./scripts/ci/pre_commit_lint_dockerfile.sh"
         files: ^Dockerfile.*$
+        pass_filenames: true
       - id: mypy
         name: Run mypy
         language: system
-        entry: ./scripts/ci/ci_mypy.sh
+        entry: "./scripts/ci/pre_commit_mypy.sh"
         files: \.py$
         exclude: ^airflow/_vendor/.*$
+        pass_filenames: true
       - id: pylint
         name: Run pylint for main sources
         language: system
-        entry: ./scripts/ci/ci_pylint_main.sh
+        entry: "./scripts/ci/pre_commit_pylint_main.sh"
         files: \.py$
         exclude: ^tests/.*\.py$|^airflow/_vendor/.*$
+        pass_filenames: true
       - id: pylint
         name: Run pylint for tests
         language: system
-        entry: ./scripts/ci/ci_pylint_tests.sh
+        entry: "./scripts/ci/pre_commit_pylint_tests.sh"
         files: ^tests/.*\.py$
+        pass_filenames: true
       - id: flake8
         name: Run flake8
         language: system
-        entry: ./scripts/ci/ci_flake8.sh
+        entry: "./scripts/ci/pre_commit_flake8.sh"
         files: \.py$
+        pass_filenames: true

.travis.yml

@@ -22,6 +22,7 @@ env:
   global:
     - BUILD_ID=${TRAVIS_BUILD_ID}
     - AIRFLOW_CONTAINER_MOUNT_HOST_VOLUMES="false"
+    - FORCE_ANSWER_TO_QUESTIONS="yes"
     - AIRFLOW_CI_SILENT="true"
     - CI="true"
 python: "3.6"

BREEZE.rst

@@ -776,6 +776,9 @@ These are the current flags of the `./breeze <./breeze>`_ script
     -y, --assume-yes
             Assume 'yes' answer to all questions.
 
+    -n, --assume-no
+            Assume 'no' answer to all questions.
+
     -C, --toggle-suppress-cheatsheet
             Toggles on/off cheatsheet displayed before starting bash shell
 
@@ -790,13 +793,13 @@ These are the current flags of the `./breeze <./breeze>`_ script
     -H, --dockerhub-repo
             DockerHub repository used to pull, push, build images. Default: airflow.
 
-    -r, --force-rebuild-images
-            Forces rebuilding of the local docker images. The images are rebuilt
+    -r, --force-build-images
+            Forces building of the local docker images. The images are rebuilt
             automatically for the first time or when changes are detected in
             package-related files, but you can force it using this flag.
 
-    -R, --force-rebuild-images-clean
-            Force rebuild images without cache. This will remove the pulled or build images
+    -R, --force-build-images-clean
+            Force build images without cache. This will remove the pulled or build images
             and start building images from scratch. This might take a long time.
 
     -p, --force-pull-images

CONTRIBUTING.md

@@ -274,7 +274,7 @@ Before running the pre-commit hooks you must first build the docker images as de
 
 Sometimes your image is outdated (when dependencies change) and needs to be rebuilt because some
 dependencies have been changed. In such case the docker build pre-commit will inform
-you that you should rebuild the image.
+you that you should build the image.
 
 ## Prerequisites for pre-commit hooks
 

Dockerfile

@@ -25,6 +25,7 @@ ARG PYTHON_BASE_IMAGE="python:3.6-slim-stretch"
 ############################################################################################################
 FROM ${PYTHON_BASE_IMAGE} as airflow-apt-deps-ci-slim
 
+
 SHELL ["/bin/bash", "-o", "pipefail", "-e", "-u", "-x", "-c"]
 
 ARG PYTHON_BASE_IMAGE="python:3.6-slim-stretch"

breeze

@@ -121,7 +121,7 @@ EOF
 export AIRFLOW_VERSION
 
 # Verbosity in running ci scripts
-export AIRFLOW_CI_VERBOSE="false"
+export VERBOSE="false"
 
 # Enter environment by default, rather than run tests or bash command or docker compose or static checks
 export RUN_TESTS="false"
@@ -130,10 +130,7 @@ export RUN_IN_BASH="false"
 export RUN_STATIC_CHECKS="false"
 export RUN_BUILD_DOCS="false"
 
-export FORCE_BUILD=${FORCE_BUILD:="false"}
-
-# if set to true, rebuild is done without asking user
-export SKIP_BUILD_CHECK="false"
+export AIRFLOW_CONTAINER_FORCE_DOCKER_BUILD=${AIRFLOW_CONTAINER_FORCE_DOCKER_BUILD:="false"}
 
 # Files determining whether asciiart/cheatsheet are suppressed
 
@@ -392,6 +389,9 @@ The swiss-knife-army tool for Airflow testings. It allows to perform various tes
 -y, --assume-yes
         Assume 'yes' answer to all questions.
 
+-n, --assume-no
+        Assume 'no' answer to all questions.
+
 -C, --toggle-suppress-cheatsheet
         Toggles on/off cheatsheet displayed before starting bash shell
 
@@ -406,13 +406,13 @@ The swiss-knife-army tool for Airflow testings. It allows to perform various tes
 -H, --dockerhub-repo
         DockerHub repository used to pull, push, build images. Default: ${_BREEZE_DEFAULT_DOCKERHUB_REPO:=}.
 
--r, --force-rebuild-images
-        Forces rebuilding of the local docker images. The images are rebuilt
+-r, --force-build-images
+        Forces building of the local docker images. The images are rebuilt
         automatically for the first time or when changes are detected in
         package-related files, but you can force it using this flag.
 
--R, --force-rebuild-images-clean
-        Force rebuild images without cache. This will remove the pulled or build images
+-R, --force-build-images-clean
+        Force build images without cache. This will remove the pulled or build images
         and start building images from scratch. This might take a long time.
 
 -p, --force-pull-images
@@ -421,7 +421,7 @@ The swiss-knife-army tool for Airflow testings. It allows to perform various tes
         environment, later the locally build images are used as cache.
 
 -u, --push-images
-        After rebuilding - uploads the images to DockerHub
+        After building - uploads the images to DockerHub
         It is useful in case you use your own DockerHub user to store images and you want
         to build them locally. Note that you need to use 'docker login' before you upload images.
 
@@ -491,22 +491,25 @@ do
       shift ;;
     -b|--build-only)
       ENTER_ENVIRONMENT="false"
-      SKIP_BUILD_CHECK="true"
-      AIRFLOW_CONTAINER_DOCKER_BUILD_NEEDED="true"
-      FORCE_BUILD="true"
+      export AIRFLOW_CONTAINER_FORCE_DOCKER_BUILD="true"
       echo "Only build. Do not enter airflow-testing container"
       echo
       shift ;;
     -v|--verbose)
-      AIRFLOW_CI_VERBOSE="true"
+      VERBOSE="true"
       echo "Verbose output"
       echo
       shift ;;
     -y|--assume-yes)
-      export ASSUME_YES="true"
+      export FORCE_ANSWER_TO_QUESTIONS="yes"
       echo "Assuming 'yes' answer to all questions."
       echo
       shift ;;
+    -n|--assume-no)
+      export FORCE_ANSWER_TO_QUESTIONS="no"
+      echo "Assuming 'no' answer to all questions."
+      echo
+      shift ;;
     -C|--toggle-suppress-cheatsheet)
       if [[ -f "${SUPPRESS_CHEATSHEET_FILE}" ]]; then
         rm -f "${SUPPRESS_CHEATSHEET_FILE}"
@@ -525,32 +528,27 @@ do
       echo "Toggle suppress asciiart"
       echo
       shift ;;
-    -r|--force-rebuild-images)
+    -r|--force-build-images)
       echo
-      echo "Force rebuild images"
+      echo "Force build images"
       echo
-      AIRFLOW_CONTAINER_DOCKER_BUILD_NEEDED="true"
-      SKIP_BUILD_CHECK="true"
-      FORCE_BUILD="true"
+      export AIRFLOW_CONTAINER_FORCE_DOCKER_BUILD="true"
       shift ;;
-    -R|--force-rebuild-images-clean)
+    -R|--force-build-images-clean)
       echo
-      echo "Clean rebuild of images without cache"
+      echo "Clean build of images without cache"
       echo
       export AIRFLOW_CONTAINER_USE_DOCKER_CACHE=false
       export AIRFLOW_CONTAINER_USE_PULLED_IMAGES_CACHE=false
-      AIRFLOW_CONTAINER_DOCKER_BUILD_NEEDED="true"
-      SKIP_BUILD_CHECK="true"
-      FORCE_BUILD="true"
+      export AIRFLOW_CONTAINER_FORCE_DOCKER_BUILD="true"
       CLEANUP_IMAGES="true"
       shift ;;
     -p|--force-pull-images)
       echo
       echo "Force pulling images before build. Uses pulled images as cache."
       echo
       export AIRFLOW_CONTAINER_FORCE_PULL_IMAGES="true"
-      AIRFLOW_CONTAINER_DOCKER_BUILD_NEEDED="true"
-      SKIP_BUILD_CHECK="true"
+      export AIRFLOW_CONTAINER_FORCE_DOCKER_BUILD="true"
       shift ;;
     -u|--push-images)
       if [[ "${AIRFLOW_FIX_PERMISSIONS}" != "all" ]]; then
@@ -568,8 +566,7 @@ do
       echo "Pushing images to DockerHub"
       echo
       export AIRFLOW_CONTAINER_PUSH_IMAGES="true"
-      AIRFLOW_CONTAINER_DOCKER_BUILD_NEEDED="true"
-      SKIP_BUILD_CHECK="true"
+      export AIRFLOW_CONTAINER_FORCE_DOCKER_BUILD="true"
       shift ;;
     -c|--cleanup-images)
       echo
@@ -683,6 +680,9 @@ echo
 
 CMDNAME="$(basename -- "$0")"
 
+# Cleans up the answer that was given last time, whether to force/
+cleanup_last_force_answer
+
 export ENV="${ENV:=$(read_from_file ENV)}"
 export BACKEND="${BACKEND:=$(read_from_file BACKEND)}"
 export KUBERNETES_VERSION="${KUBERNETES_VERSION:=$(read_from_file KUBERNETES_VERSION)}"

breeze-complete

@@ -12,7 +12,7 @@ _BREEZE_DEFAULT_DOCKERHUB_REPO="airflow"
 _BREEZE_SHORT_OPTIONS="
 h P: E: B: K: M:
 s b O
-v y C A
+v y n C A
 r R p u
 c D: H: e a
 t: d: k x: S: F:
@@ -21,8 +21,8 @@ t: d: k x: S: F:
 _BREEZE_LONG_OPTIONS="
 help python: env: backend: kubernetes-version: kubernetes-mode:
 skip-mounting-local-sources build-only build-docs
-verbose assume-yes toggle-suppress-cheatsheet toggle-suppress-asciiart
-force-rebuild-images force-rebuild-images-clean force-pull-images push-images
+verbose assume-yes assume-no toggle-suppress-cheatsheet toggle-suppress-asciiart
+force-build-images force-build-images-clean force-pull-images push-images
 cleanup-images dockerhub-user: dockerhub-repo: initialize-local-virtualenv setup-autocomplete
 test-target: docker-compose: stop-environment execute-command: static-check: static-check-all-files:
 "

confirm

@@ -17,28 +17,35 @@
 # under the License.
 set -euo pipefail
 
-if [[ "${ASSUME_YES_TO_ALL_QUESTIONS:=false}" == "true" ]]; then
-    exit 0
+if [[ "${FORCE_ANSWER_TO_QUESTIONS:=""}" != "" ]]; then
+    RESPONSE=${FORCE_ANSWER_TO_QUESTIONS}
+    echo
+    echo "Forcing response '${FORCE_ANSWER_TO_QUESTIONS}' to ${1}."
+    case "${RESPONSE}" in
+        [yY][eE][sS]|[yY])
+            exit 0 ;;
+        [qQ][uU][iI][tT]|[qQ])
+            exit 2 ;;
+        *)
+            exit 1 ;;
+    esac
+else
+    echo
+    echo "Please confirm ${1} images. Are you sure? [y/N/q]"
+    read -r RESPONSE
 fi
 
-if [[ "${ASSUME_NO_TO_ALL_QUESTIONS:=false}" == "true" ]]; then
-    exit 1
-fi
-
-if [[ "${ASSUME_QUIT_TO_ALL_QUESTIONS:=false}" == "true" ]]; then
-    exit 2
-fi
-
-
-read -r -p "${1}. Are you sure? [y/N/q] " response
-case "$response" in
+case "${RESPONSE}" in
     [yY][eE][sS]|[yY])
+        echo "The answer is 'yes'. Attempting to ${1} images. This can take some time !"
         exit 0
         ;;
-    [qQ][uU][iI|[tT]|[qQ])
+    [qQ][uU][iI][tT]|[qQ])
+        echo "The answer is 'quit'. Quitting now!"
         exit 2
         ;;
     *)
+        echo "The answer is 'no'. Skipping attempt to ${1} images."
         exit 1
         ;;
 esac