plockaby commented on a change in pull request #20147:
URL: https://github.com/apache/airflow/pull/20147#discussion_r766007391



##########
File path: airflow/task/task_runner/base_task_runner.py
##########
@@ -173,14 +179,12 @@ def terminate(self) -> None:
 
     def on_finish(self) -> None:
         """A callback that should be called when this is done running."""
-        if self._cfg_path and os.path.isfile(self._cfg_path):
-            if self.run_as_user:
-                subprocess.call(['sudo', 'rm', self._cfg_path], close_fds=True)
-            else:
+        if self.impersonation:
+            # Clean up all temporary files with sudo since they were made with
+            # sudo and the regular airflow user does not own them anymore.
+            subprocess.call(
+              ['sudo', 'rm', '-f', self._cfg_path, self._error_file.name], 
close_fds=True
+            )
+        else:
+            if self._cfg_path and os.path.isfile(self._cfg_path):
                 os.remove(self._cfg_path)
-        try:
-            self._error_file.close()
-        except FileNotFoundError:
-            # The subprocess has deleted this file before we do
-            # so we ignore

Review comment:
       Oh you're right. I glossed over this in my reading of the code.




-- 
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]


Reply via email to