Skip to content

Commit

Permalink
Speed up provider validation pre-commit (#28541)
Browse files Browse the repository at this point in the history
The provider validation pre-commit is now a lot slower after #28516
but it turned out that it has been also doing a little too much.

It worked in the way (against the original design) that when any
provider.yaml changed, validation was actually performed for all
the providers, not only for those changed.

This change improves the regular speed of running this validation
when one or few providers are changed in the commit.

It's not only limiting the validation to those provider.yaml files
that changes but also performing subset of validations on those changed
files if not all providers are changed or --all-files are not used
(because some of the validations involving documentation and
cross-provider dependencies require all provider.yaml files to be loaded
and validated).

The validation is still done in one "docker run" command though.
  • Loading branch information
potiuk authored Dec 28, 2022
1 parent 8a23bf4 commit 69df1c5
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 7 deletions.
5 changes: 2 additions & 3 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -914,12 +914,11 @@ repos:
additional_dependencies: ['rich>=12.4.4', 'inputimeout']
- id: check-provider-yaml-valid
name: Validate provider.yaml files
pass_filenames: false
entry: ./scripts/ci/pre_commit/pre_commit_check_provider_yaml_files.py
language: python
require_serial: true
files: ^docs/|provider\.yaml$|^scripts/ci/pre_commit/pre_commit_check_provider_yaml_files\.py$
files: ^airflow/providers/.*/provider.yaml$
additional_dependencies: ['rich>=12.4.4', 'inputimeout', 'markdown-it-py']
require_serial: true
- id: update-migration-references
name: Update migration ref doc
language: python
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
from __future__ import annotations

import os
import shlex
import sys
from pathlib import Path

Expand All @@ -38,6 +39,9 @@
from airflow_breeze.utils.docker_command_utils import get_extra_docker_flags
from airflow_breeze.utils.run_utils import get_ci_image_for_pre_commits, run_command

cmd = "python3 /opt/airflow/scripts/in_container/run_provider_yaml_files_check.py"
if len(sys.argv) > 1:
cmd = cmd + " " + " ".join([shlex.quote(f) for f in sys.argv[1:]])
airflow_image = get_ci_image_for_pre_commits()
cmd_result = run_command(
[
Expand All @@ -51,7 +55,7 @@
"never",
airflow_image,
"-c",
"python3 /opt/airflow/scripts/in_container/run_provider_yaml_files_check.py",
cmd,
],
check=False,
)
Expand Down
5 changes: 2 additions & 3 deletions scripts/in_container/run_provider_yaml_files_check.py
Original file line number Diff line number Diff line change
Expand Up @@ -464,9 +464,8 @@ def check_providers_have_all_documentation_files(yaml_files: dict[str, dict]):
console.print(f"Verifying packages on {architecture} architecture. Platform: {platform.machine()}.")
provider_files_pattern = pathlib.Path(ROOT_DIR).glob("airflow/providers/**/provider.yaml")
all_provider_files = sorted(str(path) for path in provider_files_pattern)

if len(sys.argv) > 1:
paths = sorted(sys.argv[1:])
paths = [os.fspath(ROOT_DIR / f) for f in sorted(sys.argv[1:])]
else:
paths = all_provider_files

Expand All @@ -484,13 +483,13 @@ def check_providers_have_all_documentation_files(yaml_files: dict[str, dict]):
check_extra_link_classes(all_parsed_yaml_files)
check_correctness_of_list_of_sensors_operators_hook_modules(all_parsed_yaml_files)
check_unique_provider_name(all_parsed_yaml_files)
check_providers_are_mentioned_in_issue_template(all_parsed_yaml_files)
check_providers_have_all_documentation_files(all_parsed_yaml_files)

if all_files_loaded:
# Only check those if all provider files are loaded
check_doc_files(all_parsed_yaml_files)
check_invalid_integration(all_parsed_yaml_files)
check_providers_are_mentioned_in_issue_template(all_parsed_yaml_files)

if errors:
console.print(f"[red]Found {len(errors)} errors in providers[/]")
Expand Down

0 comments on commit 69df1c5

Please sign in to comment.