LIU-66: Download logs from all running NM as a tar archive. #46
Conversation
daliuge-engine/dlg/manager/rest.py
Outdated
| if isinstance(res, bytes): | ||
| return res | ||
|
|
||
| if isinstance(res, bottle.HTTPResponse): | ||
| return res |
There was a problem hiding this comment.
if isinstance(res, (bytes, bottle.HTTPResponse))
daliuge-engine/dlg/manager/rest.py
Outdated
| import io | ||
| import os | ||
| import cgi | ||
| import functools | ||
| import json | ||
| import logging | ||
| import threading | ||
| import tarfile |
There was a problem hiding this comment.
This is more of a personal preference, but could we have imports alphasorted in general? It would be nicer to have this checked automatically by a tool instead of me telling it, we could do that at some point.
|
|
||
| content = [] | ||
| while True: | ||
| buffer = stream.read() |
There was a problem hiding this comment.
Wouldn't this stream.read() read all of the contents in one go? In which case there's no need for a while around it. I guess you wanted to do a buffered read here instead and accumulate into content?
There was a problem hiding this comment.
Because its a tcp stream there is no guarantee that a single read will get all the data unless there is code underneath this library that is accumulating?
There was a problem hiding this comment.
I think ill leave it for now and we can review again.
| break | ||
| content.append(buffer) | ||
|
|
||
| tar.addfile(info, io.BytesIO(initial_bytes=''.join(content).encode())) |
There was a problem hiding this comment.
Mmmm.... now that I read this: wouldn't it be possible to simply pass stream as the input of addfile and forget about collecting in content? If that's possible then you'll save some unnecessary memory copying.
| def getLogFile(self, sessionId): | ||
| fh = io.BytesIO() | ||
| with tarfile.open(fileobj=fh, mode='w:gz') as tar: | ||
| for node in self.getAllCMNodes(): |
There was a problem hiding this comment.
Maybe add a TODO here to point out that in the future we might want to fetch in this loop concurrently instead of sequentially.
LIU-66: Download logs from all running NM as a tar archive.
No description provided.