Merge lp:~soren/nova/logrotate into lp:~hudson-openstack/nova/trunk

Proposed by Soren Hansen on 2011-02-14
Status: Merged
Approved by: Vish Ishaya on 2011-02-16
Approved revision: no longer in the source branch.
Merged at revision: 684
Proposed branch: lp:~soren/nova/logrotate
Merge into: lp:~hudson-openstack/nova/trunk
Diff against target: 20 lines (+2/-2)
1 file modified
nova/log.py (+2/-2)
To merge this branch: bzr merge lp:~soren/nova/logrotate
Reviewer Review Type Date Requested Status
Vish Ishaya (community) Approve on 2011-02-16
Ryan Lucio (community) Approve on 2011-02-15
Devin Carlen (community) 2011-02-14 Approve on 2011-02-15
Review via email: mp+49723@code.launchpad.net

Commit Message

Use RotatingFileHandler instead of FileHandler.

Description of the Change

Use RotatingFileHandler instead of FileHandler.

This makes it possible to rotate log files without needing a SIGHUP or reload or anything.

To post a comment you must log in.
Ryan Lucio (rlucio) wrote :

I understand this change as you saying that the RotatingFileHandler deals with the post-rotation file offset issue. Is that correct?

Are you going to update the packaging branch logrotate files separately?

review: Needs Information
Soren Hansen (soren) wrote :

2011/2/15 Ryan Lucio <email address hidden>:
> Review: Needs Information
> I understand this change as you saying that the RotatingFileHandler deals with the post-rotation file offset issue.  Is that correct?

Yes.

> Are you going to update the packaging branch logrotate files separately?

Yes.

--
Soren Hansen        | http://linux2go.dk/
Ubuntu Developer    | http://www.ubuntu.com/
OpenStack Developer | http://www.openstack.org/

Devin Carlen (devcamcar) wrote :

lgtm

review: Approve
Ryan Lucio (rlucio) wrote :

> 2011/2/15 Ryan Lucio <email address hidden>:
> > Review: Needs Information
> > I understand this change as you saying that the RotatingFileHandler deals
> with the post-rotation file offset issue.  Is that correct?
>
> Yes.
>
> > Are you going to update the packaging branch logrotate files separately?
>
> Yes.
>
>
> --
> Soren Hansen        | http://linux2go.dk/
> Ubuntu Developer    | http://www.ubuntu.com/
> OpenStack Developer | http://www.openstack.org/

Ok. LGTM.

review: Approve
Vish Ishaya (vishvananda) wrote :

lgtm

review: Approve
OpenStack Infra (hudson-openstack) wrote :

The attempt to merge lp:~soren/nova/logrotate into lp:nova failed. Below is the output from the failed tests.

/tmp/tmpdFguAg
uid=108(jenkins) gid=65534(nogroup) groups=65534(nogroup)

Todd Willey (xtoddx) wrote :

I'm not sure we should be mangling the name of the RotatingFileHandler. I'd prefer we add it into the nova.log namespace and leave FileHandler and call it explicitly wherever it is initialized.

Soren Hansen (soren) wrote :

Addressed Todd's comment.

Vish Ishaya (vishvananda) wrote :

lgtm

review: Approve
OpenStack Infra (hudson-openstack) wrote :

Attempt to merge into lp:nova failed due to conflicts:

text conflict in nova/log.py

Vish Ishaya (vishvananda) wrote :

AFAICT, RotatingFileHandler will never rotate if you don't give it a max_bytes. Is there some kind of magic under the covers that this patch actually allows you to use normal log rotation and it will start writing to the new file?

Also, is the packaging change simply to remove the following lines from *.logrotate:

  4 postrotate
  5 restart nova-xxxx
  6 endscript

review: Needs Information
Soren Hansen (soren) wrote :

It won't automatically rotate it, but if you rename the file, it'll notice and reopen the file with the original name. So yes, we avoid the restart in logrotate.

lp:~soren/nova/logrotate updated on 2011-02-16
683. By Soren Hansen on 2011-02-16

Use a threadpool for handling requests coming in through RPC.

Vish Ishaya (vishvananda) wrote :

good deal, reapproving

review: Approve
lp:~soren/nova/logrotate updated on 2011-02-16
684. By Soren Hansen on 2011-02-16

Use RotatingFileHandler instead of FileHandler.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'nova/log.py'
2--- nova/log.py 2011-02-16 09:46:59 +0000
3+++ nova/log.py 2011-02-16 18:12:40 +0000
4@@ -94,7 +94,7 @@
5 log = logging.log
6 # handlers
7 StreamHandler = logging.StreamHandler
8-FileHandler = logging.FileHandler
9+RotatingFileHandler = logging.handlers.RotatingFileHandler
10 # logging.SysLogHandler is nicer than logging.logging.handler.SysLogHandler.
11 SysLogHandler = logging.handlers.SysLogHandler
12
13@@ -139,7 +139,7 @@
14 logging.root.addHandler(syslog)
15 logpath = get_log_file_path()
16 if logpath:
17- logfile = FileHandler(logpath)
18+ logfile = RotatingFileHandler(logpath)
19 logfile.setFormatter(_formatter)
20 logging.root.addHandler(logfile)
21