MatthewRBruce commented on a change in pull request #15490:
URL: https://github.com/apache/airflow/pull/15490#discussion_r651771863
##########
File path: airflow/providers/cncf/kubernetes/operators/kubernetes_pod.py
##########
@@ -545,17 +544,17 @@ def monitor_launched_pod(self, launcher, pod) ->
Tuple[State, Optional[str]]:
:return:
"""
try:
- (final_state, result) = launcher.monitor_pod(pod,
get_logs=self.get_logs)
+ (final_state, pod_info, result) = launcher.monitor_pod(pod,
get_logs=self.get_logs)
finally:
if self.is_delete_operator_pod:
launcher.delete_pod(pod)
if final_state != State.SUCCESS:
if self.log_events_on_failure:
- for event in launcher.read_pod_events(pod).items:
+ for event in pod_info.items:
Review comment:
I was poking at this last night and I didn't end up being overly keen on
overwriting the original pod spec in `self.pod` with the remote pod after
completion. I went back to the original idea in the PR, but I renamed all the
`pod_info` to `remote_pod` - going with the `pod` variable name as you
suggested ended up conflating the returned pod with the `pod` parameter passed
into some functions and `self.pod` which just seemed confusing.
If you wouldn't mind taking another look at the PR it would be much
appreciated :)
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
