Merge lp:~openstack-gd/nova/lp708740 into lp:~hudson-openstack/nova/trunk

[Ilya Alekseyev]

Changed default handler for uncaughted exceptions. It uses logging instead print to stderr.

[Eldar Nugaev (reldan)]

lgtm

review: Approve

[Thierry Carrez (ttx)]

That sounds like a pretty serious change in behavior to push 2 days before carving a release candidate. Could you explain to me why this needs to be pushed in Bexar ? I see that the regression risk is contained, however it changes the behavior of the software a bit too deeply, a bit too late.

review: Needs Information (gfe)

[Andrey Brindeyev (abrindeyev)]

> That sounds like a pretty serious change in behavior to push 2 days before
> carving a release candidate. Could you explain to me why this needs to be
> pushed in Bexar ? I see that the regression risk is contained, however it
> changes the behavior of the software a bit too deeply, a bit too late.

Thierry,

I already discussed that with Soren on IRC last Friday.
With current code we are loosing Python TRACEs (and all other data which Nova sents to STDOUT and STDERR) when Nova daemons properly daemonized.
See bug for details: https://bugs.launchpad.net/nova/+bug/708740

Nova daemon's startup scripts in Ubuntu needs to be slightly modified to run with our code:
s, > /var/log/nova/nova-compute.log,--logfile /var/log/nova/nova-compute.log,

[Thierry Carrez (ttx)]

Note that I don't deny the existence of a bug, I'm just trying to see if that's a bug we can release with or one that justifies a late exception.

If all it affects is currently-non-existent packaging that would make proper use of daemonization, then I don't think it's worth it. That work-in-progress packaging could even carry the necessary patch.

That said, it looks rather contained, and Soren can probably sync the fixing of Ubuntu packaging so that this does not break it. If you can get two nova-core approvals today on this one saying it's harmless, I won't oppose it.

[Vish Ishaya (vishvananda)]

I definitely would prefer to have no exceptions getting eaten. It makes debugging user issues a lot more difficult.
On Jan 31, 2011, at 5:44 AM, Thierry Carrez wrote:

> Note that I don't deny the existence of a bug, I'm just trying to see if that's a bug we can release with or one that justifies a late exception.
>
> If all it affects is currently-non-existent packaging that would make proper use of daemonization, then I don't think it's worth it. That work-in-progress packaging could even carry the necessary patch.
>
> That said, it looks rather contained, and Soren can probably sync the fixing of Ubuntu packaging so that this does not break it. If you can get two nova-core approvals today on this one saying it's harmless, I won't oppose it.
> --
> https://code.launchpad.net/~openstack-gd/nova/lp708740/+merge/47838
> You are subscribed to branch lp:nova.

[Vish Ishaya (vishvananda)]

review: Approve

[Soren Hansen (soren)]

This looks:

a) correct,
b) benign,
b) last, but not least importantly, easily revertible.

Let's do it.

review: Approve

[Soren Hansen (soren)]

Got approval from Thierry by phone (he's traveling right now).

review: Approve (gfe)

=== modified file 'nova/log.py'
--- nova/log.py	2011-01-20 14:12:02 +0000
+++ nova/log.py	2011-01-28 17:52:54 +0000
@@ -31,6 +31,7 @@
 import json
 import logging
 import logging.handlers
+import sys
 import traceback
 
 from nova import flags
@@ -191,6 +192,12 @@
             kwargs.pop('exc_info')
             self.error(message, **kwargs)
 
+
+def handle_exception(type, value, tb):
+    logging.root.critical(str(value), exc_info=(type, value, tb))
+
+
+sys.excepthook = handle_exception
 logging.setLoggerClass(NovaLogger)