Opened 4 years ago

Closed 4 years ago

#4255 enhancement closed fixed (fixed)

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
(diff, github, buildbot, log)
Author: therve Launchpad Bug:

Description (last modified by therve)

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 (14)

comment:1 Changed 4 years ago by therve

  • Description modified (diff)

comment:2 Changed 4 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.

comment:3 Changed 4 years ago by exarkun

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

comment:4 Changed 4 years ago by therve

  • Author set to therve
  • Branch set to branches/reopen-logfile-4255

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

comment:5 Changed 4 years ago by therve

  • Keywords review added
  • Owner therve deleted

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

comment:6 Changed 4 years ago by free.ekanayaka

  • Keywords review removed
  • Owner set to therve

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.

comment:7 Changed 4 years ago by therve

  • Resolution set to fixed
  • Status changed from new to closed

(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.

comment:8 Changed 4 years ago by therve

  • Resolution fixed deleted
  • Status changed from closed to reopened

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

Reopens #4255

comment:9 Changed 4 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.

comment:10 Changed 4 years ago by jkakar

  • Keywords review removed
  • Owner set to therve

[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.

comment:11 Changed 4 years ago by therve

  • Resolution set to fixed
  • Status changed from new to closed

(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.

comment:12 Changed 4 years ago by therve

  • Resolution fixed deleted
  • Status changed from closed to reopened

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

Reopens #4255

comment:13 Changed 4 years ago by therve

  • Resolution set to fixed
  • Status changed from reopened to closed

(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.

comment:14 Changed 3 years ago by <automation>

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