baha-bouali commented on code in PR #72752:
URL: https://github.com/apache/airflow/pull/72752#discussion_r4009818880
##########
airflow-core/src/airflow/config_templates/airflow_local_settings.py:
##########
@@ -152,6 +153,67 @@ def _default_conn_name_from(mod_path, hook_name):
return None
+# First provider distribution version whose ``RemoteLogIO`` exposes
``from_config`` *and*
+# registers the scheme in its provider.yaml ``remote-logging:`` block. Named
in the deprecation
+# message so a Deployment Manager knows exactly which upgrade retires the
legacy branch.
+_PROVIDER_DISPATCH_MIN_VERSIONS: dict[str, tuple[str, str]] = {
+ "s3": ("apache-airflow-providers-amazon", "9.33.0"),
+ "cloudwatch": ("apache-airflow-providers-amazon", "9.33.0"),
+ "gs": ("apache-airflow-providers-google", "22.3.0"),
+ "stackdriver": ("apache-airflow-providers-google", "22.3.0"),
+ "wasb": ("apache-airflow-providers-microsoft-azure", "14.1.0"),
+ "oss": ("apache-airflow-providers-alibaba", "3.4.0"),
+ "hdfs": ("apache-airflow-providers-apache-hdfs", "4.13.0"),
+ "elasticsearch": ("apache-airflow-providers-elasticsearch", "6.9.0"),
+ "opensearch": ("apache-airflow-providers-opensearch", "1.12.0"),
+}
+
+# Scheme of ``[logging] remote_base_log_folder``; the key ProvidersManager
dispatches on.
+# Set below when remote logging is enabled.
+_configured_scheme: str = ""
+
+
+def _warn_legacy_remote_logging(remote_log_io: type, scheme: str) -> None:
+ """
+ Warn when this legacy branch, rather than provider dispatch, is what
configures remote logging.
+
+ ``airflow.logging_config._get_logging_config`` imports this module for its
+ ``DEFAULT_LOGGING_CONFIG`` dict on every stock deployment, so the chain
below still runs
+ even when ProvidersManager scheme dispatch has already built the real
handler. Warning
+ unconditionally would therefore fire for every operator, including those
with nothing left
+ to migrate, so a branch warns only when dispatch cannot supersede it:
+
+ * the installed provider predates ``from_config``, and so registers no
scheme; or
+ * ``[logging] remote_base_log_folder`` carries no scheme to dispatch on --
a bare
+ ``wasb-logs`` path, or Elasticsearch/OpenSearch selected through their
``host`` option.
+ """
+ distribution, min_version = _PROVIDER_DISPATCH_MIN_VERSIONS[scheme]
+ provider_supports_dispatch = hasattr(remote_log_io, "from_config")
+
+ if provider_supports_dispatch and _configured_scheme == scheme:
+ return
+
+ if not provider_supports_dispatch:
+ remedy = (
+ f"Upgrade {distribution} to {min_version} or newer, which
registers the {scheme!r} "
+ f"scheme and builds this handler from
{remote_log_io.__name__}.from_config()."
+ )
+ else:
+ remedy = (
+ f"{distribution} {min_version} or newer already registers the
{scheme!r} scheme; set "
+ f'[logging] remote_base_log_folder to a "{scheme}://" URL so it is
dispatched on. '
+ f"Keep the backend options you already set, as from_config() still
reads them."
+ )
+
+ warnings.warn(
+ f"Remote logging for {scheme!r} is being configured by the if/elif
chain in "
+ f"airflow_local_settings.py. That chain is deprecated and will be
removed in Airflow 4, "
+ f"after which remote logging is resolved only through provider
registration. {remedy}",
+ RemovedInAirflow4Warning,
+ stacklevel=2,
+ )
Review Comment:
One suggestion: ES/OS migration is additive. In fact, we added
`remote_base_log_folder=elasticsearch://...` and keep the `[elasticsearch]`
section, since `from_config()` still reads `target_index`, `host_field`, etc...
from it.
It's where `remote_base_log_folder` is a router rather than a destination.
I'd suggest to cover that. Whether here (inside the warning) or in the
`ES/OS` logging docs. (I'd go with the second so the warning message stays
short).
--
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]
