Skip to content

Commit 7c7b001

Browse files
potiukpotiuk
authored
tHe output of commands of Breeze are only generated when they change (#23570)
Previously we generated output of all the commands from Breeze always, hoping that they will be the same, but rich already had two changes in the format of the SVG files which made the output different and breaking our PRs. Temporarily we pinned rich to fix the output, but better solution is to get the hash of all the configuration options and see if it changed, and only run generation when it did. This way we keep automated generation on pre-commit but we are protected from accidental change of the output. We also remove the rich limits and regenerated all svg files to ones generated by 12.4.0. Also found a way to run the check if we should run generation at all in pre-commit without prior installing breeze. Fixes: #22908
1 parent 9837e6d commit 7c7b001

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

41 files changed

+2483
-4475
lines changed

‎.pre-commit-config.yaml‎

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,7 @@ repos:
187187
exclude: ^airflow/_vendor/
188188
- id: trailing-whitespace
189189
name: Remove trailing whitespace at end of line
190-
exclude: ^airflow/_vendor/
190+
exclude: ^airflow/_vendor/|^images/breeze/output.*$
191191
- id: fix-encoding-pragma
192192
name: Remove encoding header from python files
193193
exclude: ^airflow/_vendor/
@@ -324,30 +324,30 @@ repos:
324324
files: ^setup\.cfg$|^setup\.py$
325325
pass_filenames: false
326326
entry: ./scripts/ci/pre_commit/pre_commit_check_order_setup.py
327-
additional_dependencies: ['rich==12.2.0']
327+
additional_dependencies: ['rich']
328328
- id: check-extra-packages-references
329329
name: Checks setup extra packages
330330
description: Checks if all the libraries in setup.py are listed in extra-packages-ref.rst file
331331
language: python
332332
files: ^setup\.py$|^docs/apache-airflow/extra-packages-ref\.rst$
333333
pass_filenames: false
334334
entry: ./scripts/ci/pre_commit/pre_commit_check_setup_extra_packages_ref.py
335-
additional_dependencies: ['rich==12.3.0']
335+
additional_dependencies: ['rich']
336336
# This check might be removed when min-airflow-version in providers is 2.2
337337
- id: check-airflow-2-1-compatibility
338338
name: Check that providers are 2.1 compatible.
339339
entry: ./scripts/ci/pre_commit/pre_commit_check_2_1_compatibility.py
340340
language: python
341341
pass_filenames: true
342342
files: ^airflow/providers/.*\.py$
343-
additional_dependencies: ['rich==12.3.0']
343+
additional_dependencies: ['rich']
344344
- id: update-breeze-file
345345
name: Update output of breeze commands in BREEZE.rst
346346
entry: ./scripts/ci/pre_commit/pre_commit_breeze_cmd_line.py
347347
language: python
348348
files: ^BREEZE\.rst$|^dev/breeze/.*$
349349
pass_filenames: false
350-
additional_dependencies: ['rich==12.3.0']
350+
additional_dependencies: ['rich', 'rich-click']
351351
- id: update-local-yml-file
352352
name: Update mounts in the local yml file
353353
entry: ./scripts/ci/pre_commit/pre_commit_local_yml_mounts.sh
@@ -379,7 +379,7 @@ repos:
379379
language: python
380380
files: ^Dockerfile$
381381
pass_filenames: false
382-
additional_dependencies: ['rich==12.3.0']
382+
additional_dependencies: ['rich']
383383
- id: update-supported-versions
384384
name: Updates supported versions in documentation
385385
entry: ./scripts/ci/pre_commit/pre_commit_supported_versions.py
@@ -582,15 +582,15 @@ repos:
582582
- 'jsonschema>=3.2.0,<5.0.0'
583583
- 'tabulate==0.8.8'
584584
- 'jsonpath-ng==1.5.3'
585-
- 'rich==12.3.0'
585+
- 'rich'
586586
- id: check-pre-commit-information-consistent
587587
name: Update information about pre-commit hooks and verify ids and names
588588
entry: ./scripts/ci/pre_commit/pre_commit_check_pre_commit_hooks.py
589589
args:
590590
- --max-length=70
591591
language: python
592592
files: ^\.pre-commit-config\.yaml$|^scripts/ci/pre_commit/pre_commit_check_pre_commit_hook_names\.py$
593-
additional_dependencies: ['pyyaml', 'jinja2', 'black==22.3.0', 'tabulate', 'rich==12.3.0']
593+
additional_dependencies: ['pyyaml', 'jinja2', 'black==22.3.0', 'tabulate', 'rich']
594594
require_serial: true
595595
pass_filenames: false
596596
- id: check-airflow-providers-have-extras
@@ -600,7 +600,7 @@ repos:
600600
files: ^setup\.py$|^airflow/providers/.*\.py$
601601
pass_filenames: false
602602
require_serial: true
603-
additional_dependencies: ['rich==12.3.0']
603+
additional_dependencies: ['rich']
604604
- id: update-breeze-readme-config-hash
605605
name: Update Breeze README.md with config files hash
606606
language: python
@@ -615,15 +615,15 @@ repos:
615615
files: ^dev/breeze/.*$
616616
pass_filenames: false
617617
require_serial: true
618-
additional_dependencies: ['click', 'rich==12.3.0']
618+
additional_dependencies: ['click', 'rich']
619619
- id: check-system-tests-present
620620
name: Check if system tests have required segments of code
621621
entry: ./scripts/ci/pre_commit/pre_commit_check_system_tests.py
622622
language: python
623623
files: ^tests/system/.*/example_[^/]*.py$
624624
exclude: ^tests/system/providers/google/bigquery/example_bigquery_queries\.py$
625625
pass_filenames: true
626-
additional_dependencies: ['rich==12.3.0']
626+
additional_dependencies: ['rich']
627627
- id: lint-markdown
628628
name: Run markdownlint
629629
description: Checks the style of Markdown files.
@@ -714,15 +714,15 @@ repos:
714714
language: python
715715
pass_filenames: true
716716
files: ^\.github/workflows/.*\.yml$
717-
additional_dependencies: ['PyYAML', 'rich==12.3.0']
717+
additional_dependencies: ['PyYAML', 'rich']
718718
- id: check-docstring-param-types
719719
name: Check that docstrings do not specify param types
720720
entry: ./scripts/ci/pre_commit/pre_commit_docstring_param_type.py
721721
language: python
722722
pass_filenames: true
723723
files: \.py$
724724
exclude: ^airflow/_vendor/
725-
additional_dependencies: ['rich==12.3.0']
725+
additional_dependencies: ['rich']
726726
- id: lint-chart-schema
727727
name: Lint chart/values.schema.json file
728728
entry: ./scripts/ci/pre_commit/pre_commit_chart_schema.py
@@ -757,59 +757,59 @@ repos:
757757
entry: ./scripts/ci/pre_commit/pre_commit_mypy.py
758758
files: ^dev/.*\.py$
759759
require_serial: true
760-
additional_dependencies: ['rich==12.3.0']
760+
additional_dependencies: ['rich']
761761
- id: run-mypy
762762
name: Run mypy for core
763763
language: python
764764
entry: ./scripts/ci/pre_commit/pre_commit_mypy.py --namespace-packages
765765
files: \.py$
766766
exclude: ^provider_packages|^docs|^airflow/_vendor/|^airflow/providers|^airflow/migrations|^dev
767767
require_serial: true
768-
additional_dependencies: ['rich==12.3.0']
768+
additional_dependencies: ['rich']
769769
- id: run-mypy
770770
name: Run mypy for providers
771771
language: python
772772
entry: ./scripts/ci/pre_commit/pre_commit_mypy.py --namespace-packages
773773
files: ^airflow/providers/.*\.py$
774774
require_serial: true
775-
additional_dependencies: ['rich==12.3.0']
775+
additional_dependencies: ['rich']
776776
- id: run-mypy
777777
name: Run mypy for /docs/ folder
778778
language: python
779779
entry: ./scripts/ci/pre_commit/pre_commit_mypy.py
780780
files: ^docs/.*\.py$
781781
exclude: ^docs/rtd-deprecation
782782
require_serial: true
783-
additional_dependencies: ['rich==12.3.0']
783+
additional_dependencies: ['rich']
784784
- id: run-flake8
785785
name: Run flake8
786786
language: python
787787
entry: ./scripts/ci/pre_commit/pre_commit_flake8.py
788788
files: \.py$
789789
pass_filenames: true
790790
exclude: ^airflow/_vendor/
791-
additional_dependencies: ['rich==12.3.0']
791+
additional_dependencies: ['rich']
792792
- id: lint-javascript
793793
name: ESLint against airflow/ui
794794
language: python
795795
'types_or': [javascript, tsx, ts]
796796
files: ^airflow/ui/
797797
entry: ./scripts/ci/pre_commit/pre_commit_ui_lint.py
798798
pass_filenames: false
799-
additional_dependencies: ['rich==12.3.0']
799+
additional_dependencies: ['rich']
800800
- id: lint-javascript
801801
name: ESLint against current UI JavaScript files
802802
language: python
803803
'types_or': [javascript]
804804
files: ^airflow/www/static/js/
805805
entry: ./scripts/ci/pre_commit/pre_commit_www_lint.py
806806
pass_filenames: false
807-
additional_dependencies: ['rich==12.3.0']
807+
additional_dependencies: ['rich']
808808
- id: update-migration-references
809809
name: Update migration ref doc
810810
language: python
811811
entry: ./scripts/ci/pre_commit/pre_commit_migration_reference.py
812812
pass_filenames: false
813813
files: ^airflow/migrations/versions/.*\.py$|^docs/apache-airflow/migrations-ref\.rst$
814-
additional_dependencies: ['rich==12.3.0']
814+
additional_dependencies: ['rich']
815815
## ONLY ADD PRE-COMMITS HERE THAT REQUIRE CI IMAGE

‎BREEZE.rst‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -417,6 +417,15 @@ Those are all available flags of ``config`` command:
417417
:width: 100%
418418
:alt: Breeze config
419419

420+
421+
You can also dump hash of the configuration options used - this is mostly use to generate the dump
422+
of help of the commands only when they change.
423+
424+
.. image:: ./images/breeze/output-command-hash-export.svg
425+
:width: 100%
426+
:alt: Breeze command-hash-export
427+
428+
420429
Starting complete Airflow installation
421430
======================================
422431

@@ -539,6 +548,8 @@ Configuration and maintenance
539548
* Freeing space needed to run CI tests with ``breeze free-space`` command
540549
* Fixing ownership of files in your repository with ``breeze fix-ownership`` command
541550
* Print Breeze version with ``breeze version`` command
551+
* Outputs hash of commands defined by ``breeze`` with ``command-hash-export`` (useful to avoid needless
552+
regeneration of Breeze images)
542553

543554
Release tasks
544555
-------------

‎dev/breeze/README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,6 @@ PLEASE DO NOT MODIFY THE HASH BELOW! IT IS AUTOMATICALLY UPDATED BY PRE-COMMIT.
5252

5353
---------------------------------------------------------------------------------------------------------
5454

55-
Package config hash: dfdac0e241f273bbe3ae4a3d5e9c9f39426d792c2468753d94fd0dfa6b5d46331703c5d419e5e8bf3c199129acde63cfa61e30c6a3b9a920047e0d115fd1d57c
55+
Package config hash: 7f2ce934ce9d90dcc7f97c10534416a8736cb37c1bdf10c0b9abaae77fef35875863188897e13b68899caac65072697341bd11f7cc39148b0c4774ef11a88df2
5656

5757
---------------------------------------------------------------------------------------------------------

‎dev/breeze/setup.cfg‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ install_requires =
6363
pytest-xdist
6464
pyyaml
6565
requests
66-
rich==12.3.0
66+
rich
6767
rich_click
6868

6969
[options.packages.find]

‎dev/breeze/src/airflow_breeze/commands/configuration_and_maintenance_commands.py‎

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,15 +14,17 @@
1414
# KIND, either express or implied. See the License for the
1515
# specific language governing permissions and limitations
1616
# under the License.
17-
17+
import hashlib
18+
import json
1819
import os
1920
import shutil
2021
import subprocess
2122
import sys
2223
from pathlib import Path
23-
from typing import Optional
24+
from typing import IO, Any, Dict, Optional
2425

2526
import click
27+
from click import Context
2628

2729
from airflow_breeze import NAME, VERSION
2830
from airflow_breeze.commands.main_command import main
@@ -71,6 +73,7 @@
7173
"resource-check",
7274
"free-space",
7375
"fix-ownership",
76+
"command-hash-export",
7477
"version",
7578
],
7679
}
@@ -407,6 +410,29 @@ def resource_check(verbose: bool, dry_run: bool):
407410
check_docker_resources(shell_params.airflow_image_name, verbose=verbose, dry_run=dry_run)
408411

409412

413+
def dict_hash(dictionary: Dict[str, Any]) -> str:
414+
"""MD5 hash of a dictionary. Sorted and dumped via json to account for random sequence)"""
415+
dhash = hashlib.md5()
416+
encoded = json.dumps(dictionary, sort_keys=True, default=vars).encode()
417+
dhash.update(encoded)
418+
return dhash.hexdigest()
419+
420+
421+
@main.command(
422+
name="command-hash-export",
423+
help="Outputs hash of all click commands to file or stdout if `-` "
424+
"is used (useful to see if images should be regenerated).",
425+
)
426+
@option_verbose
427+
@click.argument('output', type=click.File('wt'))
428+
def command_hash_export(verbose: bool, output: IO):
429+
with Context(main) as ctx:
430+
the_context_dict = ctx.to_info_dict()
431+
if verbose:
432+
get_console().print(the_context_dict)
433+
output.write(dict_hash(the_context_dict) + "\n")
434+
435+
410436
@main.command(name="fix-ownership", help="Fix ownership of source files to be same as host user.")
411437
@option_verbose
412438
@option_dry_run

‎dev/breeze/src/airflow_breeze/commands/release_management_commands.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@
8080
"name": "Provider verification flags",
8181
"options": [
8282
"--use-airflow-version",
83-
"--constraints-reference",
83+
"--airflow-constraints-reference",
8484
"--airflow-extras",
8585
"--use-packages-from-dist",
8686
"--package-format",

‎dev/breeze/src/airflow_breeze/utils/path_utils.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,9 @@ def find_airflow_sources_root_to_operate_on() -> Path:
211211
:return: Path for the found sources.
212212
213213
"""
214+
sources_root_from_env = os.getenv('AIRFLOW_SOURCES_ROOT', None)
215+
if sources_root_from_env:
216+
return Path(sources_root_from_env)
214217
installation_airflow_sources = get_installation_airflow_sources()
215218
if installation_airflow_sources is None and not skip_upgrade_check():
216219
get_console().print(

0 commit comments

Comments
 (0)