jason810496 commented on code in PR #72752:
URL: https://github.com/apache/airflow/pull/72752#discussion_r3996153315
##########
airflow-core/newsfragments/72752.significant.rst:
##########
@@ -0,0 +1,47 @@
+Deprecate the per-scheme ``if/elif`` chain in ``airflow_local_settings.py``
Review Comment:
Please remove the entire `airflow-core/newsfragments/72752.significant.rst`.
##########
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.
+ """
Review Comment:
Please make the docstring more concise, the purpose of the function is clear
enough.
##########
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:
Please shorten the warning message and all the hint. Additionally, we
shouldn't mention that much internal details to public users, just something
more straightforward e.g. Recommend to update ... to ... for the provider based
discovery as Airflow 4 will remove the the hardcoded discovery in core.
(perhaps need more refinement as well)
##########
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 = ""
Review Comment:
No need to add a new global variable, we can reference the existing
`remote_base_log_folder` in the function.
--
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]
