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

[Soren Hansen]

Use RotatingFileHandler instead of FileHandler.

Use RotatingFileHandler instead of FileHandler.

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

[Ryan Lucio (rlucio)]

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)]

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)]

lgtm

review: Approve

[Ryan Lucio (rlucio)]

> 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)]

lgtm

review: Approve

[OpenStack Infra (hudson-openstack)]

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)]

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)]

Addressed Todd's comment.

[Vish Ishaya (vishvananda)]

lgtm

review: Approve

[OpenStack Infra (hudson-openstack)]

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

text conflict in nova/log.py

[Vish Ishaya (vishvananda)]

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)]

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]

683. By Soren Hansen on 2011-02-16

Use a threadpool for handling requests coming in through RPC.

[Vish Ishaya (vishvananda)]

good deal, reapproving

review: Approve

[lp:~soren/nova/logrotate]

684. By Soren Hansen on 2011-02-16

Use RotatingFileHandler instead of FileHandler.

=== modified file 'nova/log.py'
--- nova/log.py	2011-02-16 09:46:59 +0000
+++ nova/log.py	2011-02-16 18:12:40 +0000
@@ -94,7 +94,7 @@
 log = logging.log
 # handlers
 StreamHandler = logging.StreamHandler
-FileHandler = logging.FileHandler
+RotatingFileHandler = logging.handlers.RotatingFileHandler
 # logging.SysLogHandler is nicer than logging.logging.handler.SysLogHandler.
 SysLogHandler = logging.handlers.SysLogHandler
 
@@ -139,7 +139,7 @@
         logging.root.addHandler(syslog)
     logpath = get_log_file_path()
     if logpath:
-        logfile = FileHandler(logpath)
+        logfile = RotatingFileHandler(logpath)
         logfile.setFormatter(_formatter)
         logging.root.addHandler(logfile)