Skip to content

Commit

Permalink
Add pre-commmit check for providers list in bug report is unique/sort…
Browse files Browse the repository at this point in the history
…ed (apache#36183)
  • Loading branch information
potiuk authored Dec 12, 2023
1 parent e9b76bc commit f8b322d
Show file tree
Hide file tree
Showing 10 changed files with 198 additions and 144 deletions.
4 changes: 1 addition & 3 deletions .github/ISSUE_TEMPLATE/airflow_providers_bug_report.yml
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ body:
- cohere
- common-io
- common-sql
- daskexecutor
- databricks
- datadog
- dbt-cloud
Expand All @@ -72,7 +71,6 @@ body:
- influxdb
- jdbc
- jenkins
- apache-kafka
- microsoft-azure
- microsoft-mssql
- microsoft-psrp
Expand All @@ -81,9 +79,9 @@ body:
- mysql
- neo4j
- odbc
- openai
- openfaas
- openlineage
- openai
- opensearch
- opsgenie
- oracle
Expand Down
7 changes: 7 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,13 @@ repos:
exclude: ^airflow/kubernetes/
entry: ./scripts/ci/pre_commit/pre_commit_check_airflow_k8s_not_used.py
additional_dependencies: ['rich>=12.4.4']
- id: check-airflow-providers-bug-report-template
name: Check airflow-bug-report provider list is sorted/unique
language: python
files: ^.github/ISSUE_TEMPLATE/airflow_providers_bug_report\.yml$
require_serial: true
entry: ./scripts/ci/pre_commit/pre_commit_check_airflow_bug_report_template.py
additional_dependencies: ['rich>=12.4.4', 'pyyaml']
- id: check-cncf-k8s-only-for-executors
name: Check cncf.kubernetes imports used for executors only
language: python
Expand Down
2 changes: 2 additions & 0 deletions STATIC_CODE_CHECKS.rst
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,8 @@ require Breeze Docker image to be built locally.
+-----------------------------------------------------------+--------------------------------------------------------------+---------+
| check-airflow-provider-compatibility | Check compatibility of Providers with Airflow | |
+-----------------------------------------------------------+--------------------------------------------------------------+---------+
| check-airflow-providers-bug-report-template | Check airflow-bug-report provider list is sorted/unique | |
+-----------------------------------------------------------+--------------------------------------------------------------+---------+
| check-apache-license-rat | Check if licenses are OK for Apache | |
+-----------------------------------------------------------+--------------------------------------------------------------+---------+
| check-base-operator-partial-arguments | Check BaseOperator and partial() arguments | |
Expand Down
1 change: 1 addition & 0 deletions dev/breeze/src/airflow_breeze/pre_commit_ids.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
"check-aiobotocore-optional",
"check-airflow-k8s-not-used",
"check-airflow-provider-compatibility",
"check-airflow-providers-bug-report-template",
"check-apache-license-rat",
"check-base-operator-partial-arguments",
"check-base-operator-usage",
Expand Down
154 changes: 79 additions & 75 deletions images/breeze/output_static-checks.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion images/breeze/output_static-checks.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
4f78b9aa5b7e62a2ceca1478900d74d9
1197108ac5d3038067e599375d5130dd
28 changes: 28 additions & 0 deletions scripts/ci/pre_commit/common_precommit_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,13 @@
from __future__ import annotations

import ast
import difflib
import os
import shlex
import shutil
import subprocess
import sys
import textwrap
from pathlib import Path

from rich.console import Console
Expand Down Expand Up @@ -148,3 +150,29 @@ def run_command_via_breeze_shell(
down_command.extend(["down", "--remove-orphans"])
subprocess.run(down_command, check=False, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL)
return result


class ConsoleDiff(difflib.Differ):
def _dump(self, tag, x, lo, hi):
"""Generate comparison results for a same-tagged range."""
for i in range(lo, hi):
if tag == "+":
yield f"[green]{tag} {x[i]}[/]"
elif tag == "-":
yield f"[red]{tag} {x[i]}[/]"
else:
yield f"{tag} {x[i]}"


def check_list_sorted(the_list: list[str], message: str, errors: list[str]) -> bool:
sorted_list = sorted(set(the_list))
if the_list == sorted_list:
console.print(f"{message} is [green]ok[/]")
console.print(the_list)
console.print()
return True
console.print(f"{message} [red]NOK[/]")
console.print(textwrap.indent("\n".join(ConsoleDiff().compare(the_list, sorted_list)), " " * 4))
console.print()
errors.append(f"ERROR in {message}. The elements are not sorted/unique.")
return False
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
#!/usr/bin/env python
#
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
from __future__ import annotations

import sys
from pathlib import Path

import yaml

sys.path.insert(0, str(Path(__file__).parent.resolve())) # make sure common_precommit_utils is imported
from common_precommit_utils import AIRFLOW_SOURCES_ROOT_PATH, check_list_sorted, console

BUG_REPORT_TEMPLATE = (
AIRFLOW_SOURCES_ROOT_PATH / ".github" / "ISSUE_TEMPLATE" / "airflow_providers_bug_report.yml"
)

DEPENDENCIES_JSON_FILE_PATH = AIRFLOW_SOURCES_ROOT_PATH / "generated" / "provider_dependencies.json"


if __name__ == "__main__":
errors: list[str] = []
template = yaml.safe_load(BUG_REPORT_TEMPLATE.read_text())
for field in template["body"]:
attributes = field.get("attributes")
if attributes:
if attributes.get("label") == "Apache Airflow Provider(s)":
check_list_sorted(attributes["options"], "Apache Airflow Provider(s)", errors)
all_providers = set(
provider.replace(".", "-")
for provider in yaml.safe_load(DEPENDENCIES_JSON_FILE_PATH.read_text()).keys()
)
for provider in set(attributes["options"]):
if provider not in all_providers:
errors.append(
f"Provider {provider} not found in provider list "
f"and still present in the template.!"
)
else:
all_providers.remove(provider)
if all_providers:
errors.append(f"Not all providers are listed: {all_providers}")
if errors:
console.print("[red]Errors found in the bug report template[/]")
for error in errors:
console.print(f"[red]{error}")
sys.exit(1)
42 changes: 9 additions & 33 deletions scripts/ci/pre_commit/pre_commit_check_order_dockerfile_extras.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,20 +21,22 @@
"""
from __future__ import annotations

import difflib
import sys
import textwrap
from pathlib import Path

from rich import print

sys.path.insert(0, str(Path(__file__).parent.resolve())) # make sure common_precommit_utils is imported
from common_precommit_utils import AIRFLOW_SOURCES_ROOT_PATH, check_list_sorted

errors: list[str] = []

MY_DIR_PATH = Path(__file__).parent.resolve()

SOURCE_DIR_PATH = MY_DIR_PATH.parents[2].resolve()
BUILD_ARGS_REF_PATH = SOURCE_DIR_PATH / "docs" / "docker-stack" / "build-arg-ref.rst"
GLOBAL_CONSTANTS_PATH = SOURCE_DIR_PATH / "dev" / "breeze" / "src" / "airflow_breeze" / "global_constants.py"
BUILD_ARGS_REF_PATH = AIRFLOW_SOURCES_ROOT_PATH / "docs" / "docker-stack" / "build-arg-ref.rst"
GLOBAL_CONSTANTS_PATH = (
AIRFLOW_SOURCES_ROOT_PATH / "dev" / "breeze" / "src" / "airflow_breeze" / "global_constants.py"
)

START_RST_LINE = ".. BEGINNING OF EXTRAS LIST UPDATED BY PRE COMMIT"
END_RST_LINE = ".. END OF EXTRAS LIST UPDATED BY PRE COMMIT"
Expand All @@ -43,32 +45,6 @@
END_PYTHON_LINE = " # END OF EXTRAS LIST UPDATED BY PRE COMMIT"


class ConsoleDiff(difflib.Differ):
def _dump(self, tag, x, lo, hi):
"""Generate comparison results for a same-tagged range."""
for i in range(lo, hi):
if tag == "+":
yield f"[green]{tag} {x[i]}[/]"
elif tag == "-":
yield f"[red]{tag} {x[i]}[/]"
else:
yield f"{tag} {x[i]}"


def _check_list_sorted(the_list: list[str], message: str) -> bool:
sorted_list = sorted(the_list)
if the_list == sorted_list:
print(f"{message} is [green]ok[/]")
print(the_list)
print()
return True
print(f"{message} [red]NOK[/]")
print(textwrap.indent("\n".join(ConsoleDiff().compare(the_list, sorted_list)), " " * 4))
print()
errors.append(f"ERROR in {message}. The elements are not sorted.")
return False


def get_replaced_content(
content: list[str],
extras_list: list[str],
Expand Down Expand Up @@ -99,12 +75,12 @@ def get_replaced_content(


def check_dockerfile():
lines = (SOURCE_DIR_PATH / "Dockerfile").read_text().splitlines()
lines = (AIRFLOW_SOURCES_ROOT_PATH / "Dockerfile").read_text().splitlines()
extras_list = None
for line in lines:
if line.startswith("ARG AIRFLOW_EXTRAS="):
extras_list = line.split("=")[1].replace('"', "").split(",")
if _check_list_sorted(extras_list, "Dockerfile's AIRFLOW_EXTRAS"):
if check_list_sorted(extras_list, "Dockerfile's AIRFLOW_EXTRAS", errors):
builds_args_content = BUILD_ARGS_REF_PATH.read_text().splitlines(keepends=True)
result = get_replaced_content(
builds_args_content,
Expand Down
40 changes: 8 additions & 32 deletions scripts/ci/pre_commit/pre_commit_check_order_setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,9 @@
"""
from __future__ import annotations

import difflib
import os
import re
import sys
import textwrap
from pathlib import Path

from rich import print
Expand All @@ -34,30 +32,8 @@

SOURCE_DIR_PATH = Path(__file__).parents[3].resolve()
sys.path.insert(0, os.fspath(SOURCE_DIR_PATH))


class ConsoleDiff(difflib.Differ):
def _dump(self, tag, x, lo, hi):
"""Generate comparison results for a same-tagged range."""
for i in range(lo, hi):
if tag == "+":
yield f"[green]{tag} {x[i]}[/]"
elif tag == "-":
yield f"[red]{tag} {x[i]}[/]"
else:
yield f"{tag} {x[i]}"


def _check_list_sorted(the_list: list[str], message: str) -> None:
sorted_list = sorted(the_list)
if the_list == sorted_list:
print(f"{message} is [green]ok[/]")
print(the_list)
print()
return
print(textwrap.indent("\n".join(ConsoleDiff().compare(the_list, sorted_list)), " " * 4))
print()
errors.append(f"ERROR in {message}. The elements are not sorted.")
sys.path.insert(0, str(Path(__file__).parent.resolve())) # make sure common_precommit_utils is imported
from common_precommit_utils import check_list_sorted


def check_main_dependent_group(setup_contents: str) -> None:
Expand All @@ -75,7 +51,7 @@ def check_main_dependent_group(setup_contents: str) -> None:
main_dependent = pattern_sub_dependent.sub(",", main_dependent_group)

src = main_dependent.strip(",").split(",")
_check_list_sorted(src, "Order of dependencies")
check_list_sorted(src, "Order of dependencies", errors)

for group in src:
check_sub_dependent_group(group)
Expand All @@ -87,7 +63,7 @@ def check_sub_dependent_group(group_name: str) -> None:
`^dependent_group_name = [.*?]\n` in setup.py
"""
print(f"[info]Checking dependency group {group_name}[/]")
_check_list_sorted(getattr(setup, group_name), f"Order of dependency group: {group_name}")
check_list_sorted(getattr(setup, group_name), f"Order of dependency group: {group_name}", errors)


def check_alias_dependent_group(setup_context: str) -> None:
Expand All @@ -101,7 +77,7 @@ def check_alias_dependent_group(setup_context: str) -> None:
for dependent in dependents:
print(f"[info]Checking alias-dependent group {dependent}[/]")
src = dependent.split(" + ")
_check_list_sorted(src, f"Order of alias dependencies group: {dependent}")
check_list_sorted(src, f"Order of alias dependencies group: {dependent}", errors)


def check_variable_order(var_name: str) -> None:
Expand All @@ -110,9 +86,9 @@ def check_variable_order(var_name: str) -> None:
var = getattr(setup, var_name)

if isinstance(var, dict):
_check_list_sorted(list(var.keys()), f"Order of dependencies in: {var_name}")
check_list_sorted(list(var.keys()), f"Order of dependencies in: {var_name}", errors)
else:
_check_list_sorted(var, f"Order of dependencies in: {var_name}")
check_list_sorted(var, f"Order of dependencies in: {var_name}", errors)


def check_install_and_setup_requires() -> None:
Expand All @@ -132,7 +108,7 @@ def check_install_and_setup_requires() -> None:
print(f"[info]Checking setup.cfg group {key}[/]")
deps = config["options"][key]
dists = [pattern_dependent_version.sub("", p) for p in deps]
_check_list_sorted(dists, f"Order of dependencies in do_setup section: {key}")
check_list_sorted(dists, f"Order of dependencies in do_setup section: {key}", errors)


if __name__ == "__main__":
Expand Down

0 comments on commit f8b322d

Please sign in to comment.