Skip to content

Commit

Permalink
Make static checks generated file more stable accross the board (#29080)
Browse files Browse the repository at this point in the history
There were couple of problems with static checks generating source
files including generated stubs in the common.sql package:

* black formatting was implemented in multiple separate scripts
  making it harded to fix problems in all of them
* generated stub files were not formatted with is_pyi=True and
  black had no way to figure it out because it was working on strings
* black formatting was not consistently applied in all places
* EOL at the end of generated stub file was missing, leading to EOL
  fixer adding them after generation leading to multiple pre-commit
  passes needed
* there was (already unused) deprecated dev dict generator that used
  its own black formatting.

There were also couple of problems with the files generated by
stubgen itself:

* Union was missing in the generated stubs (this is a known issue
  with stubgen: python/mypy#12929
* Intellij complained on Incomplete import from _typeshed missing

This PR fixes all the problems:

* black formatting is now consistenly extracted and applied everywhere
* when needed, is_pyi flag is passed to black so that it knows
  that .pyi file is being fomratted
* EOL is added at the end of file when the file is generated
* Union is added to the generated stub
* noqa is added to _typeshed import
* the dict generator is removed

As the end result, generated stub files are fully importable
(no errors reported by IntelliJ IDE) and consistently formatted
every time.

GitOrigin-RevId: 129f0820cd03c721ebebe3461489f255bb9e752c
  • Loading branch information
potiuk authored and Cloud Composer Team committed Nov 8, 2024
1 parent 6608c6c commit a8fa220
Show file tree
Hide file tree
Showing 12 changed files with 203 additions and 387 deletions.
21 changes: 11 additions & 10 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,13 @@ repos:
- --fuzzy-match-generates-todo
files: >
\.cfg$|\.conf$|\.ini$|\.ldif$|\.properties$|\.readthedocs$|\.service$|\.tf$|Dockerfile.*$
- repo: https://github.com/psf/black
rev: 22.12.0
hooks:
- id: black
name: Run black (python formatter)
args: [--config=./pyproject.toml]
exclude: ^airflow/_vendor/|^airflow/contrib/
- repo: local
hooks:
- id: update-common-sql-api-stubs
Expand Down Expand Up @@ -175,15 +182,8 @@ repos:
additional_dependencies: ['ruff==0.0.226']
files: \.pyi?$
exclude: ^airflow/_vendor/
- repo: https://github.com/psf/black
rev: 22.12.0
hooks:
- id: black
name: Run black (python formatter)
args: [--config=./pyproject.toml]
exclude: ^airflow/_vendor/|^airflow/contrib/
- repo: https://github.com/asottile/blacken-docs
rev: v1.12.1
rev: 1.13.0
hooks:
- id: blacken-docs
name: Run black on python code blocks in documentation files
Expand Down Expand Up @@ -237,7 +237,7 @@ repos:
files: ^chart/values\.schema\.json$|^chart/values_schema\.schema\.json$
pass_filenames: true
- repo: https://github.com/pre-commit/pygrep-hooks
rev: v1.9.0
rev: v1.10.0
hooks:
- id: rst-backticks
name: Check if RST files use double backticks for code
Expand All @@ -246,7 +246,7 @@ repos:
name: Check if there are no deprecate log warn
exclude: ^airflow/_vendor/
- repo: https://github.com/adrienverge/yamllint
rev: v1.28.0
rev: v1.29.0
hooks:
- id: yamllint
name: Check YAML files with yamllint
Expand Down Expand Up @@ -351,6 +351,7 @@ repos:
language: python
files: ^setup\.py$|^INSTALL$|^CONTRIBUTING\.rst$
pass_filenames: false
additional_dependencies: ['rich>=12.4.4']
- id: check-extras-order
name: Check order of extras in Dockerfile
entry: ./scripts/ci/pre_commit/pre_commit_check_order_dockerfile_extras.py
Expand Down
4 changes: 2 additions & 2 deletions airflow/providers/common/sql/operators/sql.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@
Definition of the public interface for airflow.providers.common.sql.operators.sql
isort:skip_file
"""
from _typeshed import Incomplete
from _typeshed import Incomplete # noqa: F401
from airflow.models import BaseOperator, SkipMixin
from airflow.providers.common.sql.hooks.sql import DbApiHook
from airflow.utils.context import Context
from typing import Any, Callable, Iterable, Mapping, Sequence, SupportsAbs
from typing import Any, Callable, Iterable, Mapping, Sequence, SupportsAbs, Union

def _parse_boolean(val: str) -> str | bool: ...
def parse_boolean(val: str) -> str | bool: ...
Expand Down
217 changes: 0 additions & 217 deletions dev/deprecations/generate_deprecated_dicts.py

This file was deleted.

18 changes: 3 additions & 15 deletions dev/provider_packages/prepare_provider_packages.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
import jsonschema
import rich_click as click
import semver as semver
from black import Mode, TargetVersion, format_str, parse_pyproject_toml
from packaging.version import Version
from rich.console import Console
from rich.syntax import Syntax
Expand Down Expand Up @@ -1387,29 +1388,16 @@ def update_commits_rst(


@lru_cache(maxsize=None)
def black_mode():
from black import Mode, parse_pyproject_toml, target_version_option_callback

def black_mode() -> Mode:
config = parse_pyproject_toml(os.path.join(AIRFLOW_SOURCES_ROOT_PATH, "pyproject.toml"))

target_versions = set(
target_version_option_callback(None, None, tuple(config.get("target_version", ()))),
)

target_versions = {TargetVersion[val.upper()] for val in config.get("target_version", ())}
return Mode(
target_versions=target_versions,
line_length=config.get("line_length", Mode.line_length),
is_pyi=bool(config.get("is_pyi", Mode.is_pyi)),
string_normalization=not bool(config.get("skip_string_normalization", not Mode.string_normalization)),
experimental_string_processing=bool(
config.get("experimental_string_processing", Mode.experimental_string_processing)
),
)


def black_format(content) -> str:
from black import format_str

return format_str(content, mode=black_mode())


Expand Down
44 changes: 44 additions & 0 deletions scripts/ci/pre_commit/common_precommit_black_utils.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
# 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 os
import sys
from functools import lru_cache
from pathlib import Path

from black import Mode, TargetVersion, format_str, parse_pyproject_toml

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

from common_precommit_utils import AIRFLOW_BREEZE_SOURCES_PATH # isort: skip # noqa E402


@lru_cache(maxsize=None)
def black_mode(is_pyi: bool = Mode.is_pyi) -> Mode:
config = parse_pyproject_toml(os.fspath(AIRFLOW_BREEZE_SOURCES_PATH / "pyproject.toml"))
target_versions = {TargetVersion[val.upper()] for val in config.get("target_version", ())}

return Mode(
target_versions=target_versions,
line_length=config.get("line_length", Mode.line_length),
is_pyi=is_pyi,
)


def black_format(content: str, is_pyi: bool = Mode.is_pyi) -> str:
return format_str(content, mode=black_mode(is_pyi=is_pyi))
3 changes: 2 additions & 1 deletion scripts/ci/pre_commit/common_precommit_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@
import re
from pathlib import Path

AIRFLOW_SOURCES_ROOT = Path(__file__).parents[3].resolve()
AIRFLOW_SOURCES_ROOT_PATH = Path(__file__).parents[3].resolve()
AIRFLOW_BREEZE_SOURCES_PATH = AIRFLOW_SOURCES_ROOT_PATH / "dev" / "breeze"


def filter_out_providers_on_non_main_branch(files: list[str]) -> list[str]:
Expand Down
Loading

0 comments on commit a8fa220

Please sign in to comment.