Skip to content

Commit 44e6636

Browse files
committed
Flag missing conn-fields when hook-only files change in static checks
1 parent fe9d865 commit 44e6636

4 files changed

Lines changed: 49 additions & 9 deletions

File tree

‎providers/.pre-commit-config.yaml‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,10 @@ repos:
232232
name: Validate provider.yaml files
233233
entry: ../scripts/ci/prek/check_provider_yaml_files.py
234234
language: python
235-
files: ^.*/provider\.yaml$
235+
files: >
236+
(?x)
237+
^.*/provider\.yaml$|
238+
^.*/src/airflow/providers/.*/hooks/[^/]+\.py$
236239
exclude: ^.*/.venv/.*$
237240
require_serial: true
238241
- id: check-cli-definition-imports

‎scripts/ci/prek/check_provider_conn_fields.py‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,11 @@ def check_conn_fields_for_entry(
4646
at all — the entry is then skipped entirely (no ``conn-fields`` check), or
4747
- raises any other ``Exception`` to signal an unexpected failure (converted
4848
here into an error string so callers never need to catch it).
49+
50+
When the hook returns widgets, the ``conn-fields`` section in the provider YAML
51+
must exactly match. If ``conn-fields`` is absent, every widget key is treated
52+
as missing from YAML (because any custom field missing from ``conn-fields`` will
53+
be invisible in the new React connection UI).
4954
"""
5055
hook_class_name: str = conn_type_entry["hook-class-name"]
5156
connection_type: str = conn_type_entry.get("connection-type", "?")
@@ -61,11 +66,8 @@ def check_conn_fields_for_entry(
6166
if widget_keys is None:
6267
return []
6368

64-
conn_fields = conn_type_entry.get("conn-fields")
65-
if conn_fields is None:
66-
# No conn-fields declared: the new UI simply exposes no custom fields for this
67-
# connection type, which is intentional. Nothing to validate.
68-
return []
69+
# Treat absent conn-fields as an empty dict so any hook widget is flagged as missing.
70+
conn_fields: dict = conn_type_entry.get("conn-fields") or {}
6971

7072
error = build_mismatch_error(
7173
set(conn_fields.keys()), widget_keys, connection_type, yaml_file_path, hook_class_name

‎scripts/ci/prek/check_provider_yaml_files.py‎

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
# ///
2424
from __future__ import annotations
2525

26+
import pathlib
2627
import sys
2728

2829
from common_prek_utils import (
@@ -33,11 +34,41 @@
3334

3435
initialize_breeze_prek(__name__, __file__)
3536

36-
files_to_test = sys.argv[1:]
37+
38+
def _resolve_provider_yaml_files(raw_files: list[str]) -> list[str]:
39+
"""
40+
Accept a mix of provider.yaml paths and Python source files.
41+
42+
When a Python source file is passed (e.g. a hook whose
43+
``get_connection_form_widgets()`` was edited), map it to the
44+
``provider.yaml`` at the root of the same provider package so the
45+
conn-fields check runs even when only the hook changes.
46+
47+
All paths are relative to the ``providers/`` directory, as supplied by
48+
prek. The first path segment is the provider package name
49+
(e.g. ``samba/src/airflow/...`` → ``samba/provider.yaml``).
50+
"""
51+
result: set[str] = set()
52+
for f in raw_files:
53+
p = pathlib.PurePosixPath(f)
54+
if p.name == "provider.yaml":
55+
result.add(f)
56+
else:
57+
# Map any Python file to the provider.yaml of its package root.
58+
# Path structure: <provider-pkg>/<rest...>
59+
# e.g. samba/src/airflow/providers/samba/hooks/samba.py
60+
parts = p.parts
61+
if parts:
62+
result.add(f"{parts[0]}/provider.yaml")
63+
return sorted(result)
64+
65+
66+
files_to_test = _resolve_provider_yaml_files(sys.argv[1:])
3767
cmd_result = run_command_via_breeze_run(
3868
["python3", "/opt/airflow/scripts/in_container/run_provider_yaml_files_check.py", *files_to_test],
3969
backend="sqlite",
4070
warn_image_upgrade_needed=True,
4171
extra_env={"PYTHONWARNINGS": "default"},
4272
)
4373
validate_cmd_result(cmd_result, include_ci_env_check=True)
74+

‎scripts/tests/ci/prek/test_check_conn_fields_match_form_widgets.py‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,8 +107,8 @@ class TestCheckConnFieldsForEntry:
107107
pytest.param([], _get_keys(), id="empty-on-both-sides"),
108108
pytest.param(["a"], _skip, id="skip-hook-without-get-connection-form-widgets"),
109109
pytest.param(None, _skip, id="skip-missing-conn-fields-when-hook-has-no-widgets"),
110-
# Hook with widgets but no conn-fields is allowed: new UI intentionally omits custom fields.
111-
pytest.param(None, _get_keys("field_a"), id="no-conn-fields-with-hook-widgets-is-ok"),
110+
# When hook has no custom widgets, absent conn-fields is fine.
111+
pytest.param(None, lambda _: set(), id="no-conn-fields-no-hook-widgets-is-ok"),
112112
],
113113
)
114114
def test_no_errors(self, conn_fields, get_keys):
@@ -123,6 +123,10 @@ def test_no_errors(self, conn_fields, get_keys):
123123
),
124124
pytest.param(["a"], _raise, "boom", id="unexpected-exception-message"),
125125
pytest.param(["a"], _raise, HOOK_CLASS, id="unexpected-exception-mentions-hook-class"),
126+
# absent conn-fields with hook widgets must be flagged (every widget key is "missing")
127+
pytest.param(
128+
None, _get_keys("auth_protocol"), "auth_protocol", id="absent-conn-fields-with-hook-widgets"
129+
),
126130
],
127131
)
128132
def test_one_error_containing(self, conn_fields, get_keys, expected_in_error):

0 commit comments

Comments
 (0)