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 2020/06/13 10:31:07 UTC
[GitHub] [airflow] akki commented on a change in pull request #9165: Fix DockerOperator xcom
akki commented on a change in pull request #9165:
URL: https://github.com/apache/airflow/pull/9165#discussion_r439728527
##########
File path: airflow/providers/docker/operators/docker.py
##########
@@ -243,30 +245,39 @@ def _run_image(self):
tty=self.tty,
)
- lines = self.cli.attach(container=self.container['Id'],
Review comment:
I think the [`self.cli.attach` method](https://docker-py.readthedocs.io/en/stable/api.html#docker.api.container.ContainerApiMixin.attach) with `stream=True` returns a generator which allows you to read the logs in a "streaming" fashion without using significant memory.
##########
File path: airflow/providers/docker/operators/docker.py
##########
@@ -243,30 +245,39 @@ def _run_image(self):
tty=self.tty,
)
- lines = self.cli.attach(container=self.container['Id'],
- stdout=True,
- stderr=True,
- stream=True)
+ self.cli.start(self.container["Id"])
- self.cli.start(self.container['Id'])
+ result = self.cli.wait(self.container["Id"])
+
+ lines = [
+ templ.decode("utf-8").strip() if hasattr(templ, "decode") else str(templ)
+ for templ in self.cli.logs(
+ self.container["Id"], stream=True, stdout=True, stderr=False
+ )
+ ]
Review comment:
So this will load the full logs of the application into memory, right? So if the application log file is 1 GB in size Airflow will start using 1GB extra RAM?
----------------------------------------------------------------
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:
users@infra.apache.org