msimmoteit-neozo commented on code in PR #52164:
URL: https://github.com/apache/airflow/pull/52164#discussion_r2173821267
##########
providers/cncf/kubernetes/src/airflow/providers/cncf/kubernetes/utils/pod_manager.py:
##########
@@ -524,10 +524,10 @@ def consume_logs(*, since_time: DateTime | None = None)
-> tuple[DateTime | None
message_timestamp = line_timestamp
progress_callback_lines.append(line)
else: # previous log line is complete
- for line in progress_callback_lines:
+ for progress_line in progress_callback_lines:
Review Comment:
> It's just a variable name
That is correct. And I intend for this PR to do nothing but to change this
variable name (and maybe add the unit test).
The reason this variable name needs to be changed and why I'm creating this
MR is that in line 519, the variable `line` is assigned to
`raw_line.decode("utf-8", errors="backslashreplace")` and in line 540 this
variable is used: `progress_callback_lines = [line]`. But in Python,
`for`-loops are not scoped. This means that the variable `line` is overwritten
with the last value of the loop iterator. Leading to, in my opinion, undesired
behaviour for using the `progress_callback`.
To illustrate this behaviour, I have created this Python fiddle:
[Link](https://python-fiddle.com/saved/17dfa659-d320-4393-8b81-238bfb255625)
Or you can execute this in a Python interpreter of your choice:
```py
line = "Outer line"
for line in ["Log 1", "Log 2", "Log 3"]:
x = line
print(f"Line after Loop: {line}")
def test():
f_line = "Outer Line Function"
for f_line in ["FLog 1", "FLog 2", "FLog 3"]:
y = f_line
return f_line
ret = test()
print(f"Result of Function line: {ret}")
```
And lastly, if I revert commit e1d67dd34b623bbb9bf3ce20af553f38355ad85a and
execute the unit test added for this PR, this is the result:
<img width="1130" alt="grafik"
src="https://github.com/user-attachments/assets/ab266858-71bc-4ef7-9dd2-50587fb7b9a3";
/>
--
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]
