On Tue, Jan 04, 2011 at 12:56:16AM +0200, Michael Goldish wrote:
> On 01/04/2011 12:06 AM, Eduardo Habkost wrote:
> > On Mon, Jan 03, 2011 at 07:22:17PM -0200, Eduardo Habkost wrote:
[...]
> > If you want to be extra careful, you can keep an index of contexts for
> > each exception ever seen, and clear it in _call_test_function() code,
> > instead of keeping only the last exception. But I think we can keep it
> > simple and just store info for the last exception.
>
> I think that's risky:
>
> try:
> vm.create() # context aware, raises an exception
> except:
> try:
> vm.destroy() # context aware, raises an exception
> except:
> pass
> raise
>
> The exception raised in VM.destroy() will override last_exception, and
> then whoever calls context_for_exception() will get the wrong context.
Correct.
> But I think this will work and it looks pretty clean:
>
> def _context_aware(fn, *args, **kwargs):
> new_context("")
> try:
> try:
> return fn(*args, **kwargs)
> except Exception, e:
> if not hasattr(e, "context"):
> e.context = get_context()
> raise sys.exc_info()[0], e, sys.exc_info()[2]
Wouldn't a simple "raise" (without any arguments) work here?
> finally:
> clear_context()
> This way we don't need to keep track of the last exception, and we even
> take care of the injection of .context here (instead of in test.py), so
> we don't need context_for_exception() at all.
> What do you think?
Looks good to me.
I did the 'is last_exception' trick because I assumed (incorrectly, I
guess) I couldn't set arbitrary attributes on Exception objects. I like
this solution (as long as you tested it and it works :).
> If this makes sense, I'd like to rewrite my patch using this decorator,
> unless you feel like doing so yourself.
Be my guest. :)
--
Eduardo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
