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:
[email protected]
