-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Zip archives if zippedArchive is true #1741
Conversation
* Zip archives during rotation if zippedArchive is true * Remove gzip stream which was never written to (see winstonjs#1128 (comment)) * ads tests checking the contents of zipped archives * ads tests for rolling files with tailable: false * Fixes previously failing test tests/tail.file.test.js
Tests are running without any errors on local windows with node v8, v10 and v12. Not sure why the test for the last two commit failed on appveyor, but For the first three commits the |
Can this be merged? |
👍 |
4 similar comments
👍 |
👍🏼 |
👍 |
👍🏽 |
Closing this in favor of #2337, which is essentially the same but no conflicts. Thank you for this contribution (which is mainly incorporated in the other PR -- want to make sure you're getting credit). But I think we can go ahead and try to merge this other PR and see if people are happy with it. |
As reported in #1128 since v3 the archives are not zipped anymore. They just get the ".gz" file extension.
As @glemco correctly pointed out the
gzip
stream never get's any data piped into as the log data is directly piped from the passthrough stream to the file:winston/lib/winston/transports/file.js
Line 553 in 319abf1
The docs say:
Therefore the data should not be zipped while logging, but when the files are rotated.
The PR contains the following changes and resolves #1128
(see Winston Transport File - zippedArchive option doesn't work correctly #1128 (comment))