Skip to content
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

Implemented simple ReopenLogFile #1946

Closed
wants to merge 1 commit into from

Conversation

electrofloat
Copy link

This is a base for #465

for uses cases like logrotate.
@Fishwaldo
Copy link
Member

This doesn't really do anything (especially if AppendLog is true) as it just closes and opens the log file. For rotation, we need to rename the old log file and start a new one (regardless of AppendLog) so yes - a base, but still needs some work.

@electrofloat
Copy link
Author

electrofloat commented Sep 30, 2019

This doesn't really do anything (especially if AppendLog is true) as it just closes and opens the log file.

Exactly.

For rotation, we need to rename the old log file and start a new one (regardless of AppendLog) so yes - a base, but still needs some work.

The rotation is the not job of the lib itself. Logrotate does the rotation (on linux), sends a SIGHUP to the app, the app calls ReopenLogfile in the lib, which reopens the same logfile and everything works as expected.

@Fishwaldo
Copy link
Member

Your assuming Linux and Systems with logrotate. Windows/Mac/Solaris etc etc etc may not/do not have such a thing. Hence the actual rename of the log file should be done in the library.

I appriciate the PR - I'll update it eventually. :)

@electrofloat
Copy link
Author

That is true. Since the issue was mainly talking about logrotate (#465 (comment)) and not handling SIGHUP in the lib (#465 (comment))

That is what I implemented. I'd really appreciate some kind of logrotation for the log it creates either by logrotate or by any other means.

@Fishwaldo
Copy link
Member

It just needs a rename between the close/open calls... (but needs to implemented in the platform abstraction classes so its portable).

Question begs - What do we rename to, and how do we handle duplicates etc.

I'll get to it one day :)

@Fishwaldo
Copy link
Member

Related to #2146

@Fishwaldo
Copy link
Member

Merged to Dev Branch. Thanks!

@Fishwaldo Fishwaldo closed this Mar 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants