kaxil commented on code in PR #63711:
URL: https://github.com/apache/airflow/pull/63711#discussion_r2940856382
##########
airflow-core/src/airflow/providers_manager.py:
##########
@@ -1061,6 +1061,14 @@ def _import_hook(
# inherited from parent hook. This way we add form fields only
once for the whole
# hierarchy and we add it only from the parent hook that
provides those!
if "get_connection_form_widgets" in hook_class.__dict__:
+ warnings.warn(
+ f"Hook '{hook_class_name}' defines
get_connection_form_widgets(). "
+ "This method is deprecated. Define connection fields
declaratively in "
+ "provider.yaml under 'conn-fields' instead. See "
Review Comment:
`DeprecationWarning` works, but `AirflowProviderDeprecationWarning` from
`airflow.exceptions` is the Airflow convention for deprecating provider-facing
APIs. It also has the `deprecated_provider_since` attribute which lets you
record what version introduced the deprecation. Same applies to the second
`warnings.warn` below.
This aligns with @eladkal's earlier comment.
##########
airflow-core/src/airflow/providers_manager.py:
##########
@@ -1061,6 +1061,14 @@ def _import_hook(
# inherited from parent hook. This way we add form fields only
once for the whole
# hierarchy and we add it only from the parent hook that
provides those!
if "get_connection_form_widgets" in hook_class.__dict__:
+ warnings.warn(
Review Comment:
No tests were added for the new warnings. A quick
`pytest.warns(DeprecationWarning, match="get_connection_form_widgets")` test
using a mock hook class would pin the behavior and catch accidental removal.
##########
airflow-core/src/airflow/providers_manager.py:
##########
@@ -1061,6 +1061,14 @@ def _import_hook(
# inherited from parent hook. This way we add form fields only
once for the whole
# hierarchy and we add it only from the parent hook that
provides those!
if "get_connection_form_widgets" in hook_class.__dict__:
+ warnings.warn(
+ f"Hook '{hook_class_name}' defines
get_connection_form_widgets(). "
+ "This method is deprecated. Define connection fields
declaratively in "
+ "provider.yaml under 'conn-fields' instead. See "
+
"https://airflow.apache.org/docs/apache-airflow/stable/howto/connection.html";,
+ DeprecationWarning,
+ stacklevel=2,
+ )
Review Comment:
Nit: `stacklevel=2` points to an internal `ProvidersManager` call site
(either `LazyDictWithCache.__getitem__` or
`_discover_hooks_from_hook_class_names`), not user code. Since these warnings
fire during provider loading rather than from user-initiated calls,
`stacklevel` doesn't help much here. The hook class name in the message is what
people will actually use to find the offending code, so this is fine as-is —
just noting it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
