This is an automated email from the ASF dual-hosted git repository.
potiuk pushed a commit to branch v3-3-test
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/v3-3-test by this push:
new 0446165f342 [v3-3-test] Check ui-field-behaviour drift between hooks
and provider.yaml (#73801) (#73830)
0446165f342 is described below
commit 0446165f3426102766a5d19210f988bed764898e
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Mon Sep 28 17:56:38 2026 +0200
[v3-3-test] Check ui-field-behaviour drift between hooks and provider.yaml
(#73801) (#73830)
* Flag ui-field-behaviour drift between hooks and provider.yaml in static
checks
* Share the hook lookup and clarify ui-field-behaviour drift messages
* fixup! Share the hook lookup and clarify ui-field-behaviour drift messages
Generated-by: Claude Opus 5
---------
(cherry picked from commit 2827a13dc4f5b4554cc70d09155a5baf5e3ce040)
Co-authored-by: Yuseok Jo <[email protected]>
Co-authored-by: Jarek Potiuk <[email protected]>
---
scripts/ci/prek/check_provider_conn_fields.py | 123 ++++++++++++++++++-
.../in_container/run_provider_yaml_files_check.py | 71 ++++++++---
.../test_check_ui_field_behaviour_matches_hook.py | 130 +++++++++++++++++++++
3 files changed, 306 insertions(+), 18 deletions(-)
diff --git a/scripts/ci/prek/check_provider_conn_fields.py
b/scripts/ci/prek/check_provider_conn_fields.py
index 9980095b83c..5654d315b73 100644
--- a/scripts/ci/prek/check_provider_conn_fields.py
+++ b/scripts/ci/prek/check_provider_conn_fields.py
@@ -17,7 +17,10 @@
# specific language governing permissions and limitations
# under the License.
"""
-Validation helpers for the conn-fields ↔ get_connection_form_widgets() check.
+Validation helpers for the checks that compare connection UI metadata in
provider.yaml with the hook.
+
+``conn-fields`` is compared with ``get_connection_form_widgets()``, and
``ui-field-behaviour``
+with ``get_ui_field_behaviour()``.
These functions have no third-party dependencies so they can be unit-tested
outside of the Breeze container without any stubbing.
@@ -27,6 +30,7 @@ Used by
``scripts/in_container/run_provider_yaml_files_check.py``.
from __future__ import annotations
+import json
from collections.abc import Callable
@@ -120,3 +124,120 @@ def build_mismatch_error(
)
lines.append("[yellow]How to fix it[/]: Add the missing key(s) to
conn-fields in provider.yaml.")
return "\n".join(lines)
+
+
+def normalize_behaviour_value(value: object) -> str:
+ """
+ Equality-normalize a relabeling/placeholder value.
+
+ Placeholder values are display examples, so insignificant formatting must
not count as
+ drift: surrounding whitespace is stripped, and values that parse as JSON
on both sides
+ are compared structurally (hooks often build them with ``json.dumps`` at a
different
+ indent than the YAML block scalar, and ``|`` block scalars add a trailing
newline).
+ """
+ text = str(value).strip()
+ try:
+ return json.dumps(json.loads(text), sort_keys=True)
+ except (ValueError, TypeError):
+ return text
+
+
+def normalize_placeholder_key(key: str, connection_type: str) -> str:
+ """
+ Prefix a custom-field placeholder key the way
``_ensure_prefix_for_placeholders`` does at runtime.
+
+ Airflow accepts both ``keyfile_dict`` and
``extra__<conn_type>__keyfile_dict`` for the same
+ field, so the two spellings must not count as drift.
+ """
+ if key in {"host", "schema", "login", "password", "port", "extra"} or
key.startswith("extra__"):
+ return key
+ return f"extra__{connection_type}__{key}"
+
+
+def check_ui_field_behaviour_for_entry(
+ conn_type_entry: dict,
+ yaml_file_path: str,
+ get_behaviour: Callable[[str], dict | None],
+) -> list[str]:
+ """
+ Validate a connection-type entry's ``ui-field-behaviour`` against
``get_ui_field_behaviour()``.
+
+ *get_behaviour(hook_class_name)* is a callable supplied by the caller that
returns the
+ hook's ``get_ui_field_behaviour()`` dict, or ``None`` to skip the entry
(hook not
+ importable, or it does not override the method). Any other exception is
converted to an
+ error string here so callers never need to catch it.
+
+ Airflow 3.2+ builds the connection form from the provider YAML and skips
the hook method
+ once the YAML declares connection metadata, while the older Airflow
versions a provider
+ still supports call ``get_ui_field_behaviour()``. The two must agree, or
the same form
+ looks different depending on the Airflow version. A missing
``ui-field-behaviour``
+ section is flagged too, since the hook method is deprecated in favour of
it.
+ """
+ hook_class_name: str = conn_type_entry["hook-class-name"]
+ connection_type: str = conn_type_entry.get("connection-type", "?")
+
+ try:
+ hook_behaviour = get_behaviour(hook_class_name)
+ except Exception as exc:
+ return [
+ f"Failed to call `{hook_class_name}.get_ui_field_behaviour()` "
+ f"while checking {yaml_file_path}: {exc}"
+ ]
+
+ if hook_behaviour is None:
+ return []
+
+ header = (
+ f"Mismatch between `ui-field-behaviour` in {yaml_file_path} and "
+ f"`{hook_class_name}.get_ui_field_behaviour()` "
+ f"for connection-type '{connection_type}':"
+ )
+
+ yaml_behaviour = conn_type_entry.get("ui-field-behaviour")
+ if yaml_behaviour is None:
+ return [
+ f"{header}\n"
+ " The hook overrides get_ui_field_behaviour(), which is
deprecated in favour of "
+ "ui-field-behaviour in provider.yaml, but provider.yaml has no
such section.\n"
+ "[yellow]How to fix it[/]: Declare ui-field-behaviour for this
connection-type "
+ "in provider.yaml, matching get_ui_field_behaviour()."
+ ]
+
+ problems = []
+
+ yaml_hidden = set(yaml_behaviour.get("hidden-fields") or [])
+ hook_hidden = set(hook_behaviour.get("hidden_fields") or [])
+ if yaml_hidden != hook_hidden:
+ problems.append(
+ " hidden-fields differ."
+ f" Only in provider.yaml: {sorted(yaml_hidden - hook_hidden) or
'-'};"
+ f" only in the hook: {sorted(hook_hidden - yaml_hidden) or '-'}"
+ )
+
+ key_normalizers: dict[str, Callable[[str], str]] = {
+ "relabeling": lambda key: key,
+ "placeholders": lambda key: normalize_placeholder_key(key,
connection_type),
+ }
+ for section, normalize_key in key_normalizers.items():
+ yaml_section = {
+ normalize_key(k): normalize_behaviour_value(v)
+ for k, v in (yaml_behaviour.get(section) or {}).items()
+ }
+ hook_section = {
+ normalize_key(k): normalize_behaviour_value(v)
+ for k, v in (hook_behaviour.get(section) or {}).items()
+ }
+ if yaml_section == hook_section:
+ continue
+ diff_keys = sorted(
+ k for k in yaml_section.keys() | hook_section.keys() if
yaml_section.get(k) != hook_section.get(k)
+ )
+ problems.append(f" {section} differ for: {', '.join(diff_keys)}")
+
+ if not problems:
+ return []
+ problems.append(
+ "[yellow]How to fix it[/]: Make ui-field-behaviour in provider.yaml
say the same "
+ "thing as get_ui_field_behaviour(); Airflow 3.2+ shows the YAML, older
versions the hook method."
+ )
+ return ["\n".join([header, *problems])]
diff --git a/scripts/in_container/run_provider_yaml_files_check.py
b/scripts/in_container/run_provider_yaml_files_check.py
index 9518e75ad65..3b1bb917894 100755
--- a/scripts/in_container/run_provider_yaml_files_check.py
+++ b/scripts/in_container/run_provider_yaml_files_check.py
@@ -53,7 +53,7 @@ from airflow.providers_manager import ProvidersManager
# check_provider_conn_fields lives in scripts/ci/prek/ which is not on
sys.path when
# this script runs inside Breeze; resolve it relative to this file.
sys.path.insert(0, str(pathlib.Path(__file__).parent.parent / "ci" / "prek"))
-from check_provider_conn_fields import check_conn_fields_for_entry
+from check_provider_conn_fields import check_conn_fields_for_entry,
check_ui_field_behaviour_for_entry
# Those are deprecated modules that contain removed Hooks/Sensors/Operators
that we left in the code
# so that users can get a very specific error message when they try to use
them.
@@ -500,15 +500,40 @@ def check_conn_fields_match_form_widgets(yaml_files:
dict[str, dict]) -> tuple[i
return num_checks, num_errors
-def _get_widget_keys(hook_class_name: str) -> set[str] | None:
+@run_check(
+ "Checking that ui-field-behaviour in provider.yaml matches
get_ui_field_behaviour() of the hook class"
+)
+def check_ui_field_behaviour_matches_hook(yaml_files: dict[str, dict]) ->
tuple[int, int]:
+ """
+ For every connection-type entry whose hook overrides
``get_ui_field_behaviour()``,
+ verify the ``ui-field-behaviour`` section in the provider YAML says the
same thing.
+ Airflow 3.2+ shows the YAML and older versions the hook method, so a
drifted
+ hidden-fields/relabeling/placeholders value is an error, and so is a
missing section,
+ since the hook method is deprecated in favour of it.
"""
- Import *hook_class_name* and return the keys of
``get_connection_form_widgets()``.
+ num_checks = 0
+ num_errors = 0
- Returns ``None`` when the hook or its UI dependencies cannot be imported,
- or when the hook does not override ``get_connection_form_widgets()``
(meaning it
- has no custom connection fields and the conn-fields check should be
skipped).
- Raises for unexpected errors so ``check_conn_fields_for_entry`` can
convert them
- to an error string.
+ for yaml_file_path, provider_data in yaml_files.items():
+ for conn_type_entry in provider_data.get("connection-types", []):
+ num_checks += 1
+ for error in check_ui_field_behaviour_for_entry(
+ conn_type_entry, yaml_file_path, _get_ui_field_behaviour
+ ):
+ errors.append(error)
+ num_errors += 1
+
+ return num_checks, num_errors
+
+
+def _call_overridden_hook_method(hook_class_name: str, method_name: str) ->
Any:
+ """
+ Import *hook_class_name* and return the result of calling *method_name* on
the hook class.
+
+ Returns ``None`` when the hook or its UI dependencies cannot be imported,
or when the hook
+ does not override *method_name* in its own ``__dict__``: a hook that
inherits the method has
+ no provider-specific definition to diff against, so the check is skipped
for it. Raises for
+ unexpected errors so the ``check_*_for_entry`` helpers can convert them to
an error string.
"""
try:
module_name, class_name = hook_class_name.rsplit(".", maxsplit=1)
@@ -517,23 +542,34 @@ def _get_widget_keys(hook_class_name: str) -> set[str] |
None:
except (ImportError, AirflowOptionalProviderFeatureException,
AttributeError):
return None
- # Only validate hooks that override get_connection_form_widgets() in their
own __dict__,
- # because that method is the source-of-truth for what conn-fields should
be declared.
- # Hooks that inherit it without overriding have no provider-specific
widget definition
- # to diff against, so the check is intentionally skipped for them. As of
writing this
- # includes HttpHook, the common/ai hooks, and AzureComputeHook — any
provider whose hook
- # falls into this category will NOT be validated here, even if it declares
conn-fields.
- if "get_connection_form_widgets" not in hook_class.__dict__:
+ if method_name not in hook_class.__dict__:
return None
with warnings.catch_warnings(record=True):
try:
- form_widgets: dict[str, Any] =
hook_class.get_connection_form_widgets()
- return set(form_widgets.keys())
+ return getattr(hook_class, method_name)()
except (ImportError, AirflowOptionalProviderFeatureException,
AttributeError):
return None
+def _get_ui_field_behaviour(hook_class_name: str) -> dict[str, Any] | None:
+ """Return the hook's ``get_ui_field_behaviour()`` dict, or ``None`` to
skip the entry."""
+ return _call_overridden_hook_method(hook_class_name,
"get_ui_field_behaviour")
+
+
+def _get_widget_keys(hook_class_name: str) -> set[str] | None:
+ """
+ Return the keys of the hook's ``get_connection_form_widgets()``, or
``None`` to skip the entry.
+
+ ``get_connection_form_widgets()`` is the source of truth for what
conn-fields should declare.
+ Hooks that inherit it without overriding are skipped; as of writing this
includes HttpHook,
+ the common/ai hooks, and AzureComputeHook, so any provider whose hook
falls into this
+ category will NOT be validated here, even if it declares conn-fields.
+ """
+ form_widgets = _call_overridden_hook_method(hook_class_name,
"get_connection_form_widgets")
+ return None if form_widgets is None else set(form_widgets.keys())
+
+
@run_check("Checking that hook classes defining conn_type are registered in
connection-types")
def check_hook_classes_with_conn_type_are_registered(yaml_files: dict[str,
dict]) -> tuple[int, int]:
"""Find Hook subclasses that define conn_type but are not listed in
connection-types."""
@@ -1108,6 +1144,7 @@ if __name__ == "__main__":
check_completeness_of_list_of_transfers(all_parsed_yaml_files)
check_hook_class_name_entries_in_connection_types(all_parsed_yaml_files)
check_conn_fields_match_form_widgets(all_parsed_yaml_files)
+ check_ui_field_behaviour_matches_hook(all_parsed_yaml_files)
check_hook_classes_with_conn_type_are_registered(all_parsed_yaml_files)
check_executor_classes(all_parsed_yaml_files)
check_queue_classes(all_parsed_yaml_files)
diff --git
a/scripts/tests/ci/prek/test_check_ui_field_behaviour_matches_hook.py
b/scripts/tests/ci/prek/test_check_ui_field_behaviour_matches_hook.py
new file mode 100644
index 00000000000..b00d76f793d
--- /dev/null
+++ b/scripts/tests/ci/prek/test_check_ui_field_behaviour_matches_hook.py
@@ -0,0 +1,130 @@
+# 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 pytest
+from check_provider_conn_fields import (
+ check_ui_field_behaviour_for_entry,
+ normalize_behaviour_value,
+)
+
+YAML_PATH = "providers/my_provider/provider.yaml"
+HOOK_CLASS = "my_provider.hooks.my_hook.MyHook"
+CONN_TYPE = "my_conn_type"
+
+
+def _entry(behaviour: dict | None) -> dict:
+ entry: dict = {"hook-class-name": HOOK_CLASS, "connection-type": CONN_TYPE}
+ if behaviour is not None:
+ entry["ui-field-behaviour"] = behaviour
+ return entry
+
+
+def _hook(behaviour: dict | None):
+ """Return a get_behaviour callable that always returns the given dict."""
+ return lambda _hook_class_name: behaviour
+
+
+def _raise(_hook_class_name: str) -> None:
+ raise RuntimeError("boom")
+
+
+class TestNormalizeBehaviourValue:
+ @pytest.mark.parametrize(
+ "left, right",
+ [
+ pytest.param("plain text", "plain text\n", id="trailing-newline"),
+ pytest.param(" padded ", "padded", id="surrounding-whitespace"),
+ pytest.param('{"a": 1, "b": [2]}', '{\n "a": 1,\n "b": [\n
2\n ]\n}\n', id="json-indent"),
+ pytest.param('{"b": [2], "a": 1}', '{"a": 1, "b": [2]}',
id="json-key-order"),
+ ],
+ )
+ def test_insignificant_formatting_is_equal(self, left, right):
+ assert normalize_behaviour_value(left) ==
normalize_behaviour_value(right)
+
+ @pytest.mark.parametrize(
+ "left, right",
+ [
+ pytest.param('{"a": 1}', '{"a": 2}', id="json-content"),
+ pytest.param("host url", "host url", id="inner-whitespace"),
+ ],
+ )
+ def test_real_differences_stay_different(self, left, right):
+ assert normalize_behaviour_value(left) !=
normalize_behaviour_value(right)
+
+
+class TestCheckUiFieldBehaviourForEntry:
+ @pytest.mark.parametrize(
+ "yaml_behaviour, get_behaviour",
+ [
+ pytest.param(None, _hook(None),
id="skip-hook-without-get-ui-field-behaviour"),
+ pytest.param(
+ {
+ "hidden-fields": ["port", "schema"],
+ "relabeling": {"host": "Server URL"},
+ "placeholders": {"extra": '{"a": 1, "b": 2}'},
+ },
+ _hook(
+ {
+ "hidden_fields": ["schema", "port"],
+ "relabeling": {"host": "Server URL"},
+ "placeholders": {"extra": '{\n "b": 2,\n "a":
1\n}\n'},
+ }
+ ),
+ id="matching-behaviour-with-formatting-differences",
+ ),
+ pytest.param(
+ {"placeholders": {"keyfile_dict": "{}", "login": "user"}},
+ _hook({"placeholders": {f"extra__{CONN_TYPE}__keyfile_dict":
"{}", "login": "user"}}),
+ id="bare-and-prefixed-placeholder-keys-are-equal",
+ ),
+ ],
+ )
+ def test_no_errors(self, yaml_behaviour, get_behaviour):
+ assert check_ui_field_behaviour_for_entry(_entry(yaml_behaviour),
YAML_PATH, get_behaviour) == []
+
+ @pytest.mark.parametrize(
+ "yaml_behaviour, get_behaviour, expected_in_error",
+ [
+ pytest.param(
+ None, _hook({"hidden_fields": ["port"]}), "no such section",
id="missing-yaml-section"
+ ),
+ pytest.param(
+ {"hidden-fields": ["port"]},
+ _hook({"hidden_fields": ["port", "schema"]}),
+ "only in the hook: ['schema']",
+ id="hidden-fields-drift",
+ ),
+ pytest.param(
+ {"relabeling": {"host": "Server URL"}},
+ _hook({"relabeling": {"host": "Server URL (optional)"}}),
+ "relabeling differ for: host",
+ id="relabeling-drift",
+ ),
+ pytest.param(
+ {"placeholders": {"extra": '{"model": "old"}'}},
+ _hook({"placeholders": {"extra": '{"model": "new"}', "login":
"user"}}),
+ "placeholders differ for: extra, login",
+ id="placeholders-drift",
+ ),
+ pytest.param(None, _raise, "boom",
id="unexpected-exception-message"),
+ ],
+ )
+ def test_one_error_containing(self, yaml_behaviour, get_behaviour,
expected_in_error):
+ errors = check_ui_field_behaviour_for_entry(_entry(yaml_behaviour),
YAML_PATH, get_behaviour)
+ assert len(errors) == 1
+ assert expected_in_error in errors[0]
