You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "jason810496 (via GitHub)" <gi...@apache.org> on 2026/05/17 06:26:50 UTC
[PR] Decouple remote logging config from core [airflow]
jason810496 opened a new pull request, #67056:
URL: https://github.com/apache/airflow/pull/67056
- related: https://github.com/apache/airflow/issues/65281
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Decouple remote logging config from core [airflow]
Posted by "jason810496 (via GitHub)" <gi...@apache.org>.
jason810496 commented on code in PR #67056:
URL: https://github.com/apache/airflow/pull/67056#discussion_r3263675278
##########
airflow-core/newsfragments/67056.significant.rst:
##########
@@ -0,0 +1,101 @@
+Decouple remote logging resolution from ``airflow.logging_config``
+
+Remote task log handler resolution is now owned by the shared
+``airflow_shared.logging.factory`` module and applies a single, well-defined
+precedence rule. ``airflow.logging_config.load_logging_config`` is deprecated:
+loading the ``[logging] logging_config_class`` dict and resolving the remote
+handler are now two independent steps.
+
+**Resolution order (``resolve_remote_task_log``):**
+
+1. **User-defined ``[logging] logging_config_class``** — if the user has set
+ ``logging_config_class`` to a custom module path, and that module exports a
+ ``REMOTE_TASK_LOG`` (or ``DEFAULT_REMOTE_CONN_ID``) attribute, those values
+ win. Existing custom logging configs keep working unchanged.
+2. **ProvidersManager scheme dispatch** — the scheme of ``[logging]
+ remote_base_log_folder`` (e.g. ``s3``, ``gs``, ``wasb``) is looked up in the
+ provider yaml ``remote-logging:`` registry. The matching ``RemoteLogIO``
+ class is imported and instantiated via its ``from_config()`` classmethod.
+ The connection id comes from ``[logging] remote_log_conn_id`` (set
+ explicitly by the user; providers needing a backend default can read it
+ inside ``from_config``).
+3. **Legacy attr-path fallback** — if neither of the above produced a handler,
+ the resolver imports the default logging module
+ (``airflow.config_templates.airflow_local_settings``) and reads its
+ ``REMOTE_TASK_LOG`` / ``DEFAULT_REMOTE_CONN_ID`` attributes. This is the
+ per-scheme ``if/elif`` chain in ``airflow_local_settings.py`` and is
+ transitional — it will be removed once every in-tree provider exposes
+ ``from_config``.
Review Comment:
Make sense, explicitly mention Airflow 4 in the new fragments.
##########
airflow-core/newsfragments/67056.significant.rst:
##########
@@ -0,0 +1,101 @@
+Decouple remote logging resolution from ``airflow.logging_config``
+
+Remote task log handler resolution is now owned by the shared
+``airflow_shared.logging.factory`` module and applies a single, well-defined
+precedence rule. ``airflow.logging_config.load_logging_config`` is deprecated:
+loading the ``[logging] logging_config_class`` dict and resolving the remote
+handler are now two independent steps.
+
+**Resolution order (``resolve_remote_task_log``):**
+
+1. **User-defined ``[logging] logging_config_class``** — if the user has set
+ ``logging_config_class`` to a custom module path, and that module exports a
+ ``REMOTE_TASK_LOG`` (or ``DEFAULT_REMOTE_CONN_ID``) attribute, those values
+ win. Existing custom logging configs keep working unchanged.
+2. **ProvidersManager scheme dispatch** — the scheme of ``[logging]
+ remote_base_log_folder`` (e.g. ``s3``, ``gs``, ``wasb``) is looked up in the
+ provider yaml ``remote-logging:`` registry. The matching ``RemoteLogIO``
+ class is imported and instantiated via its ``from_config()`` classmethod.
+ The connection id comes from ``[logging] remote_log_conn_id`` (set
+ explicitly by the user; providers needing a backend default can read it
+ inside ``from_config``).
+3. **Legacy attr-path fallback** — if neither of the above produced a handler,
+ the resolver imports the default logging module
+ (``airflow.config_templates.airflow_local_settings``) and reads its
+ ``REMOTE_TASK_LOG`` / ``DEFAULT_REMOTE_CONN_ID`` attributes. This is the
+ per-scheme ``if/elif`` chain in ``airflow_local_settings.py`` and is
+ transitional — it will be removed once every in-tree provider exposes
+ ``from_config``.
Review Comment:
Make sense, explicitly mention Airflow 4 in the newfragments.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Decouple remote logging config from core [airflow]
Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on code in PR #67056:
URL: https://github.com/apache/airflow/pull/67056#discussion_r3261275066
##########
airflow-core/src/airflow/providers_manager.py:
##########
@@ -1240,6 +1256,32 @@ def _discover_logging(self) -> None:
if _correctness_check(provider_package, logging_class_name, provider):
self._logging_class_name_set.add(logging_class_name)
+ def _discover_remote_logging(self) -> None:
+ """Retrieve all remote logging IO handlers defined in the providers."""
+ for provider_package, provider in self._provider_dict.items():
+ entries = provider.data.get("remote-logging") or []
+ for entry in entries:
+ classpath = entry["classpath"]
+ if not _correctness_check(provider_package, classpath, provider):
+ continue
+ info = RemoteLoggingInfo(
+ classpath=classpath,
+ scheme=entry["scheme"],
+ package_name=provider_package,
+ )
+ self._remote_logging_info_list.append(info)
+ existing = self._remote_logging_by_scheme.get(info.scheme)
+ if existing is not None:
+ log.warning(
+ "Remote logging scheme '%s' is already registered by %s; ignoring "
+ "duplicate registration from %s.",
+ info.scheme,
+ existing.package_name,
+ info.package_name,
+ )
+ continue
+ self._remote_logging_by_scheme[info.scheme] = info
Review Comment:
Pessimistic, I think if we ignore on duplicate scheme we should also only add it to list if check passed
```suggestion
existing = self._remote_logging_by_scheme.get(info.scheme)
if existing is not None:
log.warning(
"Remote logging scheme '%s' is already registered by %s; ignoring "
"duplicate registration from %s.",
info.scheme,
existing.package_name,
info.package_name,
)
continue
self._remote_logging_info_list.append(info)
self._remote_logging_by_scheme[info.scheme] = info
```
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Decouple remote logging config from core [airflow]
Posted by "jason810496 (via GitHub)" <gi...@apache.org>.
jason810496 commented on code in PR #67056:
URL: https://github.com/apache/airflow/pull/67056#discussion_r3263671272
##########
airflow-core/src/airflow/providers_manager.py:
##########
@@ -1240,6 +1256,32 @@ def _discover_logging(self) -> None:
if _correctness_check(provider_package, logging_class_name, provider):
self._logging_class_name_set.add(logging_class_name)
+ def _discover_remote_logging(self) -> None:
+ """Retrieve all remote logging IO handlers defined in the providers."""
+ for provider_package, provider in self._provider_dict.items():
+ entries = provider.data.get("remote-logging") or []
+ for entry in entries:
+ classpath = entry["classpath"]
+ if not _correctness_check(provider_package, classpath, provider):
+ continue
+ info = RemoteLoggingInfo(
+ classpath=classpath,
+ scheme=entry["scheme"],
+ package_name=provider_package,
+ )
+ self._remote_logging_info_list.append(info)
+ existing = self._remote_logging_by_scheme.get(info.scheme)
+ if existing is not None:
+ log.warning(
+ "Remote logging scheme '%s' is already registered by %s; ignoring "
+ "duplicate registration from %s.",
+ info.scheme,
+ existing.package_name,
+ info.package_name,
+ )
+ continue
+ self._remote_logging_by_scheme[info.scheme] = info
Review Comment:
Nice catch! Addressed in https://github.com/apache/airflow/pull/67056/changes/37939dca6bf004b45a8b78d82da0550e48b20b03
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Decouple remote logging config from core [airflow]
Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on code in PR #67056:
URL: https://github.com/apache/airflow/pull/67056#discussion_r3261236467
##########
airflow-core/newsfragments/67056.significant.rst:
##########
@@ -0,0 +1,101 @@
+Decouple remote logging resolution from ``airflow.logging_config``
+
+Remote task log handler resolution is now owned by the shared
+``airflow_shared.logging.factory`` module and applies a single, well-defined
+precedence rule. ``airflow.logging_config.load_logging_config`` is deprecated:
+loading the ``[logging] logging_config_class`` dict and resolving the remote
+handler are now two independent steps.
+
+**Resolution order (``resolve_remote_task_log``):**
+
+1. **User-defined ``[logging] logging_config_class``** — if the user has set
+ ``logging_config_class`` to a custom module path, and that module exports a
+ ``REMOTE_TASK_LOG`` (or ``DEFAULT_REMOTE_CONN_ID``) attribute, those values
+ win. Existing custom logging configs keep working unchanged.
+2. **ProvidersManager scheme dispatch** — the scheme of ``[logging]
+ remote_base_log_folder`` (e.g. ``s3``, ``gs``, ``wasb``) is looked up in the
+ provider yaml ``remote-logging:`` registry. The matching ``RemoteLogIO``
+ class is imported and instantiated via its ``from_config()`` classmethod.
+ The connection id comes from ``[logging] remote_log_conn_id`` (set
+ explicitly by the user; providers needing a backend default can read it
+ inside ``from_config``).
+3. **Legacy attr-path fallback** — if neither of the above produced a handler,
+ the resolver imports the default logging module
+ (``airflow.config_templates.airflow_local_settings``) and reads its
+ ``REMOTE_TASK_LOG`` / ``DEFAULT_REMOTE_CONN_ID`` attributes. This is the
+ per-scheme ``if/elif`` chain in ``airflow_local_settings.py`` and is
+ transitional — it will be removed once every in-tree provider exposes
+ ``from_config``.
Review Comment:
Probably as our semver promise we can deprecate but should keep it in until Airflow 4
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Decouple remote logging config from core [airflow]
Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl merged PR #67056:
URL: https://github.com/apache/airflow/pull/67056
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org