Convert remaining f-string logger calls to lazy form#2680
Conversation
Follow-up to #2672. The original sweep caught ~47 single-line logger.X(f"...") sites but missed two patterns: 1. Multi-line calls of the form logger.exception( f"..." ) in cosmos/plugin/airflow3.py (three sites) and cosmos/dbt/parser/output.py (one site). The original regex required the f-string on the same line as the logger call. 2. self.log.X(f"...") calls in operators that inherit Airflow's LoggingMixin. These never matched the original logger.X(f"...") pattern even though they violate the lazy logging rule identically. Affects azure_container_instance, docker, kubernetes, aws_ecs, gcp_cloud_run_job, and virtualenv operators. Also converts one eager percent-interpolation call in cosmos/cache.py ("%s" % repr(e)) that used %-formatting instead of an f-string and so was outside the scope of #2672. No behaviour change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR completes the follow-up sweep to convert remaining eager log message formatting (f-strings and %-interpolation) into lazy logging calls across Cosmos, including multiline logger.exception(...) patterns and self.log.*(...) usages from Airflow LoggingMixin.
Changes:
- Convert multiline
logger.exception(f"...")calls to lazylogger.exception("...", args...)form. - Convert
self.log.<level>(f"...")calls in several operators to lazy formatting. - Replace one eager string interpolation in
cosmos/cache.pywith lazy%rformatting.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
cosmos/plugin/airflow3.py |
Converts multiline logger.exception(f"...") calls to lazy logging with args. |
cosmos/operators/virtualenv.py |
Converts several self.log.info(f"...") calls to lazy logging. |
cosmos/operators/kubernetes.py |
Converts command logging to lazy form. |
cosmos/operators/gcp_cloud_run_job.py |
Converts command logging to lazy form. |
cosmos/operators/docker.py |
Converts command logging to lazy form. |
cosmos/operators/azure_container_instance.py |
Converts command logging to lazy form. |
cosmos/operators/aws_ecs.py |
Converts command logging to lazy form. |
cosmos/dbt/parser/output.py |
Converts an f-string error log to lazy logging with args. |
cosmos/cache.py |
Replaces eager %-interpolation with lazy logging using %r. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2680 +/- ##
==========================================
+ Coverage 98.03% 98.07% +0.03%
==========================================
Files 105 105
Lines 7843 7843
==========================================
+ Hits 7689 7692 +3
+ Misses 154 151 -3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
tatiana
left a comment
There was a problem hiding this comment.
@pankajastro thanks for addressing this. Please, could you adapt our CLAUDE.md or AGENTS.md to instruct agents to use logging with lazy form instead of f-string?
We have added this claude.md in PR #2679 |
The Logging section told contributors to "get loggers via `cosmos.log.get_logger`" unconditionally, which would lead someone working in an operator to add a module-level `get_logger` — the exact inconsistency #1157 set out to fix. Scope that guidance to library/module-level code and state the operator rule: inside operators and hooks (anything with `LoggingMixin`), log via `self.log` so messages land in the per-task-instance log shown in the Airflow UI. Examples updated to show both cases. This locks in the outcome of the logging cleanup (#2670, #2680, #2681) so it doesn't silently regress.
Follow-up to #2672. The original sweep caught ~47 single-line logger.X(f"...") sites but missed two patterns:
Multi-line calls of the form
in cosmos/plugin/airflow3.py (three sites) and cosmos/dbt/parser/output.py (one site). The original regex required the f-string on the same line as the logger call.
self.log.X(f"...") calls in operators that inherit Airflow's LoggingMixin. These never matched the original logger.X(f"...") pattern even though they violate the lazy logging rule identically. Affects azure_container_instance, docker, kubernetes, aws_ecs, gcp_cloud_run_job, and virtualenv operators.
Also converts one eager percent-interpolation call in cosmos/cache.py ("%s" % repr(e)) that used %-formatting instead of an f-string and so was outside the scope of #2672.
No behaviour change.