You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2022/04/22 09:56:17 UTC

[GitHub] [airflow] zengbotang commented on a diff in pull request #23160: Always -f when removing container after DockerOperator execution

zengbotang commented on code in PR #23160:
URL: https://github.com/apache/airflow/pull/23160#discussion_r856022649


##########
airflow/providers/docker/operators/docker.py:
##########
@@ -333,7 +333,7 @@ def _run_image_with_mounts(
             return None
         finally:
             if self.auto_remove:
-                self.cli.remove_container(self.container['Id'])
+                self.cli.remove_container(self.container['Id'], force=True)

Review Comment:
   @uranusjr Thanks for the review, there may be misunderstandings here,
   
   Imagine two scenarios:
   1. The dagrun is healthy, the container failed.
   2.dagrun failed, but the container is healthy and still running.
   
   I think What you described above is Scenario 1, what I came across is Scenario 2.
   
   For scenario 1, the current code is able to remove the container because the container is failed.
   
   But in scenario 2, the container will not be remove because the container is still running.
   
   For scenario 2, I think there is no need to debug the container, because the container is healthy.



-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org