Ticket #4255 enhancement closed fixed

Opened 3 years ago

Last modified 3 years ago

BaseLogFile should provide a method to reopen logs

Reported by: therve Owned by:
Priority: normal Milestone:
Component: core Keywords:
Cc: Branch: branches/reopen-logfile-4255
Author: therve Launchpad Bug:

Description (last modified by therve) (diff)

When using an external tool to rotate logs like logrotate, we need to ask the twisted log files to be reopened. BaseLogFile is missing a method to do that, because it involves accessing the private method _openFile.

Change History

1

Changed 3 years ago by therve

  • description modified (diff)

2

Changed 3 years ago by radix

It'd be nice to actually implement a signal handler to reopen log files in twistd itself, as well. Maybe that should be a separate ticket.

3

Changed 3 years ago by exarkun

That would be nice, and it probably should be a separate ticket.

4

Changed 3 years ago by therve

  • branch set to branches/reopen-logfile-4255
  • branch_author set to therve

(In [28260]) Branching to 'reopen-logfile-4255'

5

Changed 3 years ago by therve

  • owner therve deleted
  • keywords review added

This is ready to review! Nothing surprising, I hope.

6

Changed 3 years ago by free.ekanayaka

  • owner set to therve
  • keywords review removed

Nice fix. If it's not already opened it would be nice to file a ticket for the signal handler.

[1]

+ f = open(self.path, "r") + self.assertEquals(f.read(), "hello2") + f.close()

I'm not sure if trial has it already, but maybe it would be nice to have an "assertFileContent" convenience to perform this kind of assertion.

7

Changed 3 years ago by therve

  • status changed from new to closed
  • resolution set to fixed

(In [28311]) Merge reopen-logfile-4255

Author: therve Reviewer: free.ekanayaka Fixes: #4255

Add a reopen method to BaseLogfile to allow using external log rotation.

8

Changed 3 years ago by therve

  • status changed from closed to reopened
  • resolution fixed deleted

(In [28315]) Revert r28311: regressions under Windows

Reopens #4255

9

Changed 3 years ago by therve

  • keywords review added
  • owner therve deleted
  • status changed from reopened to new

There were some problems with Windows, but the test should be skipped now.

10

Changed 3 years ago by jkakar

  • owner set to therve
  • keywords review removed

[1]

+ L[logfile.LogFile.reopen} allows to rename the currently used file and

The L[ needs to be L{ here.

I can't test that the test is skipped on win32, but it looks fine.

11

Changed 3 years ago by therve

  • status changed from new to closed
  • resolution set to fixed

(In [28334]) Merge reopen-logfile-4255

Author: therve Reviewers: free.ekanayaka, jkakar Fixes: #4255

Add a reopen method to BaseLogfile to allow using external log rotation. This re-merge fix a test failure under Windows.

12

Changed 3 years ago by therve

  • status changed from closed to reopened
  • resolution fixed deleted

(In [28339]) Revert r28334: forget one review comment.

Reopens #4255

13

Changed 3 years ago by therve

  • status changed from reopened to closed
  • resolution set to fixed

(In [28341]) Merge reopen-logfile-4255

Author: therve Reviewers: free.ekanayaka, jkakar Fixes: #4255

Add a reopen method to BaseLogfile to allow using external log rotation. This re-merge fix a test failure under Window and a missed review.

14

Changed 2 years ago by <automation>

  • owner therve deleted
Note: See TracTickets for help on using tickets.