Re-enables MyPy in non-failure mode (apache#19890)

This PR enables mypy back as pre-commit for local changes after
the apache#19317 switched to Python 3.7 but also it separates out
mypy to a separate non-failing step in CI.

In the CI we will be able to see remaining mypy errors.

This will allow us to gradually fix all the mypy errors and enable
mypy back when we got all the problems fixed.

Author: potiuk

.github/workflows/ci.yml

@@ -429,7 +429,6 @@ jobs:
     needs: [build-info, ci-images]
     env:
       RUNS_ON: ${{ fromJson(needs.build-info.outputs.runsOn) }}
-      SKIP: "identity"
       MOUNT_SELECTED_LOCAL_SOURCES: "true"
       PYTHON_MAJOR_MINOR_VERSION: ${{needs.build-info.outputs.defaultPythonVersion}}
     if: needs.build-info.outputs.basic-checks-only == 'false'
@@ -471,10 +470,17 @@ ${{ hashFiles('.pre-commit-config.yaml') }}"
         with:
           path: 'airflow/ui/node_modules'
           key: ${{ runner.os }}-ui-node-modules-${{ hashFiles('airflow/ui/**/yarn.lock') }}
-      - name: "Static checks"
+      - name: "Static checks (no mypy)"
         run: ./scripts/ci/static_checks/run_static_checks.sh
         env:
           VERBOSE: false
+          SKIP: "identity,mypy"
+      - name: "MyPy static checks"
+        run: ./scripts/ci/static_checks/run_static_checks.sh mypy
+        env:
+          VERBOSE: false
+          DO_NOT_FAIL_ON_ERROR: "true"
+          COLUMNS: 300
 
   # Those checks are run if no image needs to be built for checks. This is for simple changes that
   # Do not touch any of the python code or any of the important files that might require building

.pre-commit-config.yaml

@@ -721,22 +721,23 @@ repos:
       - id: mypy
         name: Run mypy
         language: system
-        entry: ./scripts/ci/pre_commit/pre_commit_mypy.sh
+        entry: ./scripts/ci/pre_commit/pre_commit_mypy.sh --namespace-packages
         files: \.py$
-        exclude: ^dev|^provider_packages|^chart|^docs|^airflow/_vendor/
+        exclude: ^provider_packages|^chart|^docs|^airflow/_vendor/
+        require_serial: true
       - id: mypy
         name: Run mypy for helm chart tests
         language: system
         entry: ./scripts/ci/pre_commit/pre_commit_mypy.sh
         files: ^chart/.*\.py$
-        require_serial: false
+        require_serial: true
       - id: mypy
         name: Run mypy for /docs/ folder
         language: system
         entry: ./scripts/ci/pre_commit/pre_commit_mypy.sh
         files: ^docs/.*\.py$
         exclude: rtd-deprecation
-        require_serial: false
+        require_serial: true
       - id: flake8
         name: Run flake8
         language: system

scripts/ci/pre_commit/pre_commit_check_pre_commit_hook_names.py

@@ -27,7 +27,7 @@
 try:
     from yaml import CSafeLoader as SafeLoader
 except ImportError:
-    from yaml import SafeLoader  # type: ignore[no-redef]
+    from yaml import SafeLoader  # type: ignore
 
 
 def main() -> int:

scripts/ci/pre_commit/pre_commit_check_provider_yaml_files.py

@@ -23,7 +23,7 @@
 from collections import Counter
 from glob import glob
 from itertools import chain, product
-from typing import Any, Dict, Iterable, List
+from typing import Any, Dict, Iterable, List, Set
 
 import jsonschema
 import yaml
@@ -34,7 +34,7 @@
 try:
     from yaml import CSafeLoader as SafeLoader
 except ImportError:
-    from yaml import SafeLoader  # type: ignore[no-redef]
+    from yaml import SafeLoader  # type: ignore
 
 if __name__ != "__main__":
     raise Exception(
@@ -80,7 +80,7 @@ def _load_package_data(package_paths: Iterable[str]):
     return result
 
 
-def get_all_integration_names(yaml_files):
+def get_all_integration_names(yaml_files) -> List[str]:
     all_integrations = [
         i['integration-name'] for f in yaml_files.values() if 'integrations' in f for i in f["integrations"]
     ]
@@ -137,7 +137,7 @@ def assert_sets_equal(set1, set2):
 
 
 def check_if_objects_belongs_to_package(
-    object_names: List[str], provider_package: str, yaml_file_path: str, resource_type: str
+    object_names: Set[str], provider_package: str, yaml_file_path: str, resource_type: str
 ):
     for object_name in object_names:
         if not object_name.startswith(provider_package):
@@ -283,8 +283,8 @@ def check_invalid_integration(yaml_files: Dict[str, Dict]):
 
 def check_doc_files(yaml_files: Dict[str, Dict]):
     print("Checking doc files")
-    current_doc_urls = []
-    current_logo_urls = []
+    current_doc_urls: List[str] = []
+    current_logo_urls: List[str] = []
     for provider in yaml_files.values():
         if 'integrations' in provider:
             current_doc_urls.extend(

scripts/ci/pre_commit/pre_commit_json_schema.py

@@ -149,6 +149,8 @@ def _default_validator(validator, default, instance, schema):
 def _load_spec(spec_file: Optional[str], spec_url: Optional[str]):
     if spec_url:
         spec_file = fetch_and_cache(url=spec_url, output_filename=re.sub(r"[^a-zA-Z0-9]", "-", spec_url))
+    if not spec_file:
+        raise Exception(f"The {spec_file} was None and {spec_url} did not lead to any file loading.")
     with open(spec_file) as schema_file:
         schema = json.loads(schema_file.read())
     return schema

scripts/ci/pre_commit/pre_commit_mypy.sh

@@ -20,8 +20,5 @@ export FORCE_ANSWER_TO_QUESTIONS=${FORCE_ANSWER_TO_QUESTIONS:="quit"}
 export REMEMBER_LAST_ANSWER="true"
 export PRINT_INFO_FROM_SCRIPTS="false"
 
-# Temporarily remove mypy checks until we fix them for Python 3.7
-exit 0
-
 # shellcheck source=scripts/ci/static_checks/mypy.sh
 . "$( dirname "${BASH_SOURCE[0]}" )/../static_checks/mypy.sh" "${@}"

scripts/ci/static_checks/mypy.sh

@@ -26,7 +26,7 @@ function run_mypy() {
       files=("$@")
     fi
 
-    docker_v run "${EXTRA_DOCKER_FLAGS[@]}" \
+    docker_v run "${EXTRA_DOCKER_FLAGS[@]}" -t \
         --entrypoint "/usr/local/bin/dumb-init"  \
         "-v" "${AIRFLOW_SOURCES}/.mypy_cache:/opt/airflow/.mypy_cache" \
         "${AIRFLOW_CI_IMAGE_WITH_TAG}" \

scripts/ci/static_checks/run_static_checks.sh

@@ -34,6 +34,13 @@ python -m pip install --user pre-commit \
 
 export PATH=~/.local/bin:${PATH}
 
+if [[ ${DO_NOT_FAIL_ON_ERROR=} != "" ]]; then
+    echo
+    echo "Skip failing on error"
+    echo
+    set +e
+fi
+
 if [[ $# == "0" ]]; then
     pre-commit run --all-files --show-diff-on-failure --color always
 else
@@ -42,3 +49,7 @@ else
         pre-commit run "${pre_commit_check}" --all-files --show-diff-on-failure --color always
     done
 fi
+
+if [[ ${DO_NOT_FAIL_ON_ERROR=} != "" ]]; then
+    exit 0
+fi

scripts/in_container/run_mypy.sh

@@ -20,18 +20,4 @@
 . "$( dirname "${BASH_SOURCE[0]}" )/_in_container_script_init.sh"
 export PYTHONPATH=${AIRFLOW_SOURCES}
 
-mypy_args=()
-
-# Mypy doesn't cope very well with namespace packages when give filenames (it gets confused about the lack of __init__.py in airflow.providers, and thinks airflow.providers.docker is the same as a "docker" top level module).
-#
-# So we instead need to convert the file names in to module names to check
-for filename in "$@";
-do
-    if [[ "${filename}" == docs/* ]]; then
-        mypy_args+=("$filename")
-    else
-        mypy_args+=("-m" "$(filename_to_python_module "$filename")")
-    fi
-done
-
-mypy --namespace-packages "${mypy_args[@]}"
+mypy "${@}"

setup.py

@@ -513,6 +513,31 @@ def write_version(filename: str = os.path.join(*[my_dir, "airflow", "git_version
 ]
 # End dependencies group
 
+# Mypy 0.900 and above ships only with stubs from stdlib so if we need other stubs, we need to install them
+# manually as `types-*`. See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
+# for details. Wy want to install them explicitly because we want to eventually move to
+# mypyd which does not support installing the types dynamically with --install-types
+mypy_dependencies = [
+    'mypy==0.910',
+    'types-croniter',
+    'types-docutils',
+    'types-freezegun',
+    'types-paramiko',
+    'types-protobuf',
+    'types-python-dateutil',
+    'types-python-slugify',
+    'types-pytz',
+    'types-redis',
+    'types-requests',
+    'types-setuptools',
+    'types-termcolor',
+    'types-tabulate',
+    'types-Markdown',
+    'types-PyMySQL',
+    'types-PyYAML',
+]
+
+# Dependencies needed for development only
 devel_only = [
     'aws_xray_sdk',
     'beautifulsoup4~=4.7.1',
@@ -533,7 +558,6 @@ def write_version(filename: str = os.path.join(*[my_dir, "airflow", "git_version
     'jsondiff',
     'mongomock',
     'moto~=2.2, >=2.2.12',
-    'mypy==0.770',
     'parameterized',
     'paramiko',
     'pipdeptree',
@@ -558,7 +582,7 @@ def write_version(filename: str = os.path.join(*[my_dir, "airflow", "git_version
     'yamllint',
 ]
 
-devel = cgroups + devel_only + doc + kubernetes + mysql + pandas + password
+devel = cgroups + devel_only + doc + kubernetes + mypy_dependencies + mysql + pandas + password
 devel_hadoop = devel + hdfs + hive + kerberos + presto + webhdfs
 
 # Dict of all providers which are part of the Apache Airflow repository together with their requirements
@@ -893,6 +917,10 @@ def get_all_provider_packages() -> str:
 class AirflowDistribution(Distribution):
     """The setuptools.Distribution subclass with Airflow specific behaviour"""
 
+    def __init__(self, attrs=None):
+        super().__init__(attrs)
+        self.install_requires = None
+
     def parse_config_files(self, *args, **kwargs) -> None:
         """
         Ensure that when we have been asked to install providers from sources
@@ -989,7 +1017,7 @@ def add_all_provider_packages() -> None:
 class Develop(develop_orig):
     """Forces removal of providers in editable mode."""
 
-    def run(self) -> None:
+    def run(self) -> None:  # type: ignore
         self.announce('Installing in editable mode. Uninstalling provider packages!', level=log.INFO)
         # We need to run "python3 -m pip" because it might be that older PIP binary is in the path
         # And it results with an error when running pip directly (cannot import pip module)
@@ -1052,11 +1080,11 @@ def include_provider_namespace_packages_when_installing_from_sources() -> None:
             'extra_clean': CleanCommand,
             'compile_assets': CompileAssets,
             'list_extras': ListExtras,
-            'install': Install,
+            'install': Install,  # type: ignore
             'develop': Develop,
         },
         test_suite='setup.airflow_test_suite',
-        **setup_kwargs,
+        **setup_kwargs,  # type: ignore
     )