diff --git a/contributing-docs/13_airflow_dependencies_and_extras.rst b/contributing-docs/13_airflow_dependencies_and_extras.rst index 28ebdb67ba5d0..5cf64d2060fa0 100644 --- a/contributing-docs/13_airflow_dependencies_and_extras.rst +++ b/contributing-docs/13_airflow_dependencies_and_extras.rst @@ -213,6 +213,15 @@ rules to remember: ``# use next version`` comment to it, next time when ``prek hook`` will be run it will remove the automatically added line and keep only the manually added line with the comment. +# There is an automated check for that in case of ``common.compat`` provider - because it is one that changes + often and almost always when it changes, there are some providers that need those changes - so you should + add the ``# use next version`` comment to such providers always when you modify ``common.compat``. + In case there is a change in the ``common.compat`` package that requires updating other providers, and there + are no changes in other providers that require such update - you will get and error in the Selective Check + CI job that will remind you to add such comments to the providers that need to be updated. You can skip + the check if you are sure that no other providers do not need to be updated by adding + ``skip common compat check`` label to the PR. Only maintainers and collaborators can add such label. + * Some of our dependencies have forced minimum version - mostly because of the Airflow 3 minimum version compatibility. Just in case in the future, we have other distributions referring to them we are forcing a minimum version for those distributions by a ``prek`` hook. This causes entries like this: diff --git a/dev/breeze/src/airflow_breeze/utils/selective_checks.py b/dev/breeze/src/airflow_breeze/utils/selective_checks.py index ee00d4dff4601..1213b6dc1c853 100644 --- a/dev/breeze/src/airflow_breeze/utils/selective_checks.py +++ b/dev/breeze/src/airflow_breeze/utils/selective_checks.py @@ -88,6 +88,7 @@ USE_PUBLIC_RUNNERS_LABEL = "use public runners" ALLOW_TRANSACTION_CHANGE_LABEL = "allow translation change" ALLOW_PROVIDER_DEPENDENCY_BUMP_LABEL = "allow provider dependency bump" +SKIP_COMMON_COMPAT_CHECK_LABEL = "skip common compat check" ALL_CI_SELECTIVE_TEST_TYPES = "API Always CLI Core Other Serialization" ALL_PROVIDERS_SELECTIVE_TEST_TYPES = ( @@ -1767,3 +1768,93 @@ def parse_dep(dep_str: str) -> tuple[str, str | None]: ) return violations + + def _has_common_compat_changed(self) -> bool: + """Check if any common.compat provider file was changed.""" + return any(f.startswith("providers/common/compat/") for f in self._files) + + def _get_changed_providers_excluding_common_compat(self) -> set[str]: + """Get set of changed providers excluding common.compat itself.""" + changed_providers: set[str] = set() + for changed_file in self._files: + provider = find_provider_affected(changed_file, include_docs=False) + if provider and provider not in ["common.compat", "Providers"]: + changed_providers.add(provider) + return changed_providers + + def _uses_next_version_comment(self, provider: str) -> bool: + """Check if provider's pyproject.toml has '# use next version' for common-compat dependency.""" + pyproject_file = f"providers/{provider.replace('.', '/')}/pyproject.toml" + result = run_command( + ["git", "show", f"{self._commit_ref}:{pyproject_file}"], + capture_output=True, + text=True, + cwd=AIRFLOW_ROOT_PATH, + check=False, + ) + if result.returncode != 0: + return True # If file doesn't exist, don't flag as violation + + # Check if dependency line contains both the package and the comment + for line in result.stdout.splitlines(): + if "apache-airflow-providers-common-compat" in line.lower(): + return "# use next version" in line.lower() + return True # If dependency not found, don't flag as violation + + def _print_violations_and_exit_or_bypass(self, violations: list[str]) -> bool: + """Print violations and either exit with error or bypass with warning.""" + console = get_console() + + if SKIP_COMMON_COMPAT_CHECK_LABEL in self._pr_labels: + console.print("[warning]The 'skip common compat check' label is set. Bypassing check for:[/]") + for provider in violations: + console.print( + f"[warning] - {provider} (providers/{provider.replace('.', '/')}/pyproject.toml)[/]" + ) + console.print() + return True + + console.print( + "[error]common.compat provider changed but the following providers don't have " + "'# use next version' comment for their common-compat dependency![/]" + ) + console.print() + for provider in violations: + console.print(f"[error] - {provider} (providers/{provider.replace('.', '/')}/pyproject.toml)[/]") + console.print() + console.print( + "[warning]When common.compat changes with other providers in the same PR, " + "add '# use next version' comment where they depend on common-compat.[/]\n" + "[warning]Example:[/] " + '[info]"apache-airflow-providers-common-compat>=1.8.0", # use next version[/]\n' + ) + console.print( + f"[warning]To bypass this check, add the label: '[info]{SKIP_COMMON_COMPAT_CHECK_LABEL}[/]'\n" + ) + sys.exit(1) + + @cached_property + def common_compat_changed_without_next_version(self) -> bool: + """ + Check if common.compat provider changed and other providers changed don't have '# use next version' + comment for their common-compat dependency. + """ + if self._github_event != GithubEvents.PULL_REQUEST: + return False + + if not self._has_common_compat_changed(): + return False + + changed_providers = self._get_changed_providers_excluding_common_compat() + if not changed_providers: + return False # Only common.compat changed + + get_console().print(f"[warning]common.compat changed with providers: {sorted(changed_providers)}[/]") + + # Find providers missing '# use next version' comment + violations = [p for p in sorted(changed_providers) if not self._uses_next_version_comment(p)] + + if violations: + return self._print_violations_and_exit_or_bypass(violations) + + return False diff --git a/dev/breeze/tests/test_selective_checks.py b/dev/breeze/tests/test_selective_checks.py index b642563a352d3..9376131dd25fc 100644 --- a/dev/breeze/tests/test_selective_checks.py +++ b/dev/breeze/tests/test_selective_checks.py @@ -3099,3 +3099,163 @@ def test_large_pr_by_line_count(files, git_diff_output, expected_outputs: dict[s default_branch="main", ) assert_outputs_are_printed(expected_outputs, str(stderr)) + + +@patch("airflow_breeze.utils.selective_checks.run_command") +def test_common_compat_changed_with_next_version_passes(mock_run_command): + """Test that check passes when common.compat changes and other provider has '# use next version'.""" + provider_toml = """ +[project] +dependencies = [ + "apache-airflow>=2.11.0", + "apache-airflow-providers-common-compat>=1.8.0", # use next version +] +""" + + def side_effect(*args, **kwargs): + result = Mock() + result.returncode = 0 + result.stdout = provider_toml + return result + + mock_run_command.side_effect = side_effect + + selective_checks = SelectiveChecks( + files=( + "providers/common/compat/src/airflow/providers/common/compat/file.py", + "providers/ftp/src/airflow/providers/ftp/hooks/ftp.py", + ), + commit_ref=NEUTRAL_COMMIT, + pr_labels=(), + github_event=GithubEvents.PULL_REQUEST, + default_branch="main", + ) + result = selective_checks.common_compat_changed_without_next_version + assert result is False + + +@patch("airflow_breeze.utils.selective_checks.run_command") +def test_common_compat_changed_without_next_version_fails(mock_run_command): + """Test that check fails when common.compat changes and other provider doesn't have '# use next version'.""" + provider_toml = """ +[project] +dependencies = [ + "apache-airflow>=2.11.0", + "apache-airflow-providers-common-compat>=1.8.0", +] +""" + + def side_effect(*args, **kwargs): + result = Mock() + result.returncode = 0 + result.stdout = provider_toml + return result + + mock_run_command.side_effect = side_effect + + with pytest.raises(SystemExit): + _ = SelectiveChecks( + files=( + "providers/common/compat/src/airflow/providers/common/compat/file.py", + "providers/ftp/src/airflow/providers/ftp/hooks/ftp.py", + ), + commit_ref=NEUTRAL_COMMIT, + pr_labels=(), + github_event=GithubEvents.PULL_REQUEST, + default_branch="main", + ).common_compat_changed_without_next_version + + +@patch("airflow_breeze.utils.selective_checks.run_command") +def test_common_compat_only_changed_passes(mock_run_command): + """Test that check passes when only common.compat provider changes.""" + selective_checks = SelectiveChecks( + files=("providers/common/compat/src/airflow/providers/common/compat/file.py",), + commit_ref=NEUTRAL_COMMIT, + pr_labels=(), + github_event=GithubEvents.PULL_REQUEST, + default_branch="main", + ) + result = selective_checks.common_compat_changed_without_next_version + assert result is False + + +@patch("airflow_breeze.utils.selective_checks.run_command") +def test_common_compat_not_changed_passes(mock_run_command): + """Test that check passes when common.compat provider doesn't change.""" + selective_checks = SelectiveChecks( + files=("providers/ftp/src/airflow/providers/ftp/hooks/ftp.py",), + commit_ref=NEUTRAL_COMMIT, + pr_labels=(), + github_event=GithubEvents.PULL_REQUEST, + default_branch="main", + ) + result = selective_checks.common_compat_changed_without_next_version + assert result is False + + +@patch("airflow_breeze.utils.selective_checks.run_command") +def test_common_compat_changed_with_provider_without_dependency_passes(mock_run_command): + """Test that check passes when other provider doesn't depend on common-compat.""" + provider_toml = """ +[project] +dependencies = [ + "apache-airflow>=2.11.0", + "some-other-package>=1.0.0", +] +""" + + def side_effect(*args, **kwargs): + result = Mock() + result.returncode = 0 + result.stdout = provider_toml + return result + + mock_run_command.side_effect = side_effect + + selective_checks = SelectiveChecks( + files=( + "providers/common/compat/src/airflow/providers/common/compat/file.py", + "providers/ftp/src/airflow/providers/ftp/hooks/ftp.py", + ), + commit_ref=NEUTRAL_COMMIT, + pr_labels=(), + github_event=GithubEvents.PULL_REQUEST, + default_branch="main", + ) + result = selective_checks.common_compat_changed_without_next_version + assert result is False + + +@patch("airflow_breeze.utils.selective_checks.run_command") +def test_common_compat_changed_without_next_version_bypassed_with_label(mock_run_command): + """Test that check can be bypassed with 'skip common compat check' label.""" + provider_toml = """ +[project] +dependencies = [ + "apache-airflow>=2.11.0", + "apache-airflow-providers-common-compat>=1.8.0", +] +""" + + def side_effect(*args, **kwargs): + result = Mock() + result.returncode = 0 + result.stdout = provider_toml + return result + + mock_run_command.side_effect = side_effect + + selective_checks = SelectiveChecks( + files=( + "providers/common/compat/src/airflow/providers/common/compat/file.py", + "providers/ftp/src/airflow/providers/ftp/hooks/ftp.py", + ), + commit_ref=NEUTRAL_COMMIT, + pr_labels=("skip common compat check",), + github_event=GithubEvents.PULL_REQUEST, + default_branch="main", + ) + # Should pass with the skip label + result = selective_checks.common_compat_changed_without_next_version + assert result is True