Feature/413 limit deployment log file size #736
Conversation
There was a problem hiding this comment.
Can the three classes DeploymentLog, DeploymentLogContent and DeploymentLogContent be merged into one class? Why does the id type differ per class (int, Integer and long)?
There was a problem hiding this comment.
Hmm... there seems to be only one DeploymentLogContent - did you mean DeploymentLogContentCommand?
There was a problem hiding this comment.
I merged DeploymentLog and DeploymentLogContent and cleaned up the various types.
DeploymentLogContentCommand is used to encapsulate the input validation.
| String name = logsPath + File.separator + logName; | ||
| File file = new File(name); | ||
|
|
||
| if (file.length() > 1_000_000) throw new IOException(String.format("%s is larger than 10 MB and cannot be shown.", file.getName())); |
There was a problem hiding this comment.
Instead of an exception, could we instead read the first 10MB of the file and append an error to it?
There was a problem hiding this comment.
We could also try to stream the file instead to prevent loading it into memory completely.
There was a problem hiding this comment.
Both are good suggestions - I'll have to check it. I wonder if the users browser will crash if we stream large files.
There was a problem hiding this comment.
I've chosen a third option:
Only the last 10mb of the log file is shown (if larger) and a message is prepended that says that some part of the log is missing...
I imagine that the interesting log entries are at the end of the log - and not at the beginning.
@yvespp What do you think about it?
mburri
force-pushed
the
feature/413-limit-deployment-log-file-size
branch
from
7733494 to
b653133
Compare
… a message if content was trunated
yvespp
force-pushed
the
feature/413-limit-deployment-log-file-size
branch
from
5f413ff to
5644dbc
Compare
To be merged after #735
fixes #413