[AIRFLOW-5369] Add interactivity to pre-commits

[potiuk]

Make sure you have checked all steps below.

Jira

• My PR addresses the following Airflow Jira issues and references them in the PR title. For example, "[AIRFLOW-XXX] My Airflow PR"
  • https://issues.apache.org/jira/browse/AIRFLOW-5369

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.

Description

• Here are some details about my PR, including screenshots of any UI changes:

Tests

• My PR adds the following unit tests OR does not need testing for this extremely good reason:
  No tests for bash scripts (maybe we should add them eventually!)

Commits

• My commits all reference Jira issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "How to write a good git commit message":
  1. Subject is separated from body by a blank line
  2. Subject is limited to 50 characters (not including Jira issue reference)
  3. Subject does not end with a period
  4. Subject uses the imperative mood ("add", not "adding")
  5. Body wraps at 72 characters
  6. Body explains "what" and "why", not "how"

Documentation

• In case of new functionality, my PR adds documentation that describes how to use it.
  • All the public functions and the classes in the PR contain docstrings that explain what it does
  • If you implement backwards incompatible changes, please leave a note in the Updating.md so we can assign it to a appropriate release

Code Quality

• Passes flake8

[potiuk]

This one is really useful if you have pre-commit installed and use it.

It integrates well with breeze not only wit pre-commits - it will now let you answer y/n/q when pre-commit detects that the files need rebuilding.

If you answer "no" it will run all tests in pre-commit using the old, previously built images (which in vast majority of cases will be ok). This is really useful for offline cases (now you can run all pre-commits in offline mode easily and when we also merge #5972 it will be really fast and no necessary rebuilds when you switch between branches and you will be able to use full power of consistent pre-commits without waiting too long for occasional image rebuilds.

[potiuk]

This one is more difficult/longer to review but I think it might help a lot with adoption and regular use of pre-commit. I myself started using --no-verify far too often, but after we merge #5972 and this one, it should be really smooth to use pre-commit daily (@mik-laj @nuclearpinguin can confirm that pre-commits are really great for daily routine I think).

[dimberman]

@potiuk Is there any way to break this into smaller PRs? There's just... a lot going on here...

[potiuk]

@dimberman . You are totally right! It started from interactivity but I added a number of other small fixes along the way and now it's too big /complex/has a lot of unrelated changes.

I will break it.

[potiuk]

Hey @dimberman - I moved out the dumb-init change, and I tried to split it even further, but it seem what's left is totally related. I needed to make a bit more consistent behaviour when I ask interactively whether to rebuild the images. Now we have a really nice behaviour that there are no more failures when you have out-of-date images but it is all interactive.

BTW. I am seriously thinking about adding unit tests for the bash scripts already ;).

[potiuk]

@mik-laj -> that also addresses your recent questions about rebuilding the images when you run pre-commit manually. I thought this change is already merged, but it was not.

[codecov-io]

Codecov Report

Merging #5976 into master will decrease coverage by 0.13%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##           master   #5976      +/-   ##
=========================================
- Coverage   80.03%   79.9%   -0.14%     
=========================================
  Files         607     607              
  Lines       35019   35019              
=========================================
- Hits        28029   27983      -46     
- Misses       6990    7036      +46

Impacted Files	Coverage Δ
airflow/executors/sequential_executor.py	47.61% <0%> (-52.39%)	⬇️
airflow/utils/log/colored_log.py	81.81% <0%> (-11.37%)	⬇️
airflow/utils/sqlalchemy.py	86.44% <0%> (-6.78%)	⬇️
airflow/executors/__init__.py	63.26% <0%> (-4.09%)	⬇️
airflow/utils/dag_processing.py	56.44% <0%> (-2.36%)	⬇️
airflow/jobs/scheduler_job.py	73.08% <0%> (-1.21%)	⬇️
airflow/models/taskinstance.py	93.22% <0%> (-0.51%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fa8e18a...46a22e7. Read the comment docs.

[potiuk]

Hey @dimberman -> it's still big but it adds really nice feature to pre-commits now. It's fully interactive now and when you need to rebuild an image, it will ask you - so that you can make decision. Just tested it and it is really nice to work with it.

[potiuk]

It would be great to get this merged before the workshop in Guadalahara :)

[potiuk]

Hey @dimberman :)?

[potiuk]

🙏

[mik-laj]

@potiuk Can you do rebase?