Skip to content

Conversation

@Jason-Y-Z
Copy link
Contributor

@Jason-Y-Z Jason-Y-Z commented Mar 4, 2024

…ormatting twice

This stops us from formatting the new record twice in RotatingFileHandler.

@serhiy-storchaka
Copy link
Member

@vsajip, what are your thoughts about this?

Comment on lines +6151 to +6152
self.assertFalse(rh.shouldRollover(self.next_rec()))
rh.emit(self.next_rec())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

self.next_rec() returns a new record. You should save it and pass the same record to shouldRollover() and emit().

@Jason-Y-Z Jason-Y-Z closed this Aug 3, 2024
@Jason-Y-Z Jason-Y-Z deleted the fix-issue-116263 branch August 3, 2024 19:40
@vsajip
Copy link
Member

vsajip commented Aug 5, 2024

I'm not sure why @Jason-Y-Z closed this - maybe due to impatience due to my lack of response. I've not had much time to look at this, but I was thinking about trying a different approach - caching the formatted value in an instance variable to avoid reformatting. Since everything happens inside emit(), in theory one could avoid coupling by replacing the FileHandler.emit(...) call with just the stream.write(...) and self.flush() that StreamHandler.emit() does (since self.stream won't be None at that point, FileHandler.emit() is essentially the same as StreamHandler.emit(...) - it's just two lines of code duplication, and so doesn't seem unreasonable to do to avoid coupling. Thoughts?

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.

3 participants