ashb commented on code in PR #60258:
URL: https://github.com/apache/airflow/pull/60258#discussion_r2671818556
##########
shared/logging/src/airflow_shared/logging/percent_formatter.py:
##########
@@ -53,17 +53,22 @@ def __getitem__(self, key):
# If there is no callsite info (often for stdout/stderr), show the
same sort of thing that stdlib
# logging would
#
https://github.com/python/cpython/blob/d3c888b4ec15dbd7d6b6ef4f15b558af77c228af/Lib/logging/__init__.py#L1652C34-L1652C48
- if key == "lineno":
- return self.event.get("lineno", 0)
- if key == "filename":
- return self.event.get("filename", "(unknown file)")
- if key == "funcName":
- return self.event.get("funcName", "(unknown function)")
-
if key in PercentFormatRender.callsite_parameters:
- return
self.event.get(PercentFormatRender.callsite_parameters[key].value)
+ val =
self.event.get(PercentFormatRender.callsite_parameters[key].value)
+ # Return appropriate default based on the parameter type
+ if val is None:
+ if key == "lineno":
+ return 0
+ if key in ("process", "thread"):
+ return 0
+ if key == "filename":
+ return "(unknown file)"
+ if key == "funcName":
+ return "(unknown function)"
+ return "(unknown)"
+ return val
Review Comment:
Why did you change it to this approach -- having 3 levels of indenting makes
this harder to understand and follow.
And do we really need the `return "(unkown)"` fallback given there are only
a predefined list of percent strings? Is it possible to hit?
##########
shared/logging/src/airflow_shared/logging/percent_formatter.py:
##########
@@ -53,17 +53,22 @@ def __getitem__(self, key):
# If there is no callsite info (often for stdout/stderr), show the
same sort of thing that stdlib
# logging would
#
https://github.com/python/cpython/blob/d3c888b4ec15dbd7d6b6ef4f15b558af77c228af/Lib/logging/__init__.py#L1652C34-L1652C48
- if key == "lineno":
- return self.event.get("lineno", 0)
- if key == "filename":
- return self.event.get("filename", "(unknown file)")
- if key == "funcName":
- return self.event.get("funcName", "(unknown function)")
-
if key in PercentFormatRender.callsite_parameters:
- return
self.event.get(PercentFormatRender.callsite_parameters[key].value)
+ val =
self.event.get(PercentFormatRender.callsite_parameters[key].value)
+ # Return appropriate default based on the parameter type
+ if val is None:
+ if key == "lineno":
+ return 0
+ if key in ("process", "thread"):
+ return 0
+ if key == "filename":
+ return "(unknown file)"
+ if key == "funcName":
+ return "(unknown function)"
+ return "(unknown)"
+ return val
Review Comment:
Wont this also mean the the "name" fallback of looking at `logger` thebn
`logger_name` isn't respected anymore?
--
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]
