#399 Allow nesting BaseHandler exception handler decorators.
Merged by jkaluza. Opened by jkaluza.
jkaluza/freshmaker nesting-decorators  into  master

Download 399.patch

Previously, when multiple decorated methods were nested and
one of them raised an exception, the exception has been handled
by all the decorators.

This is not right way, because exception which should be handled
just by the fail_artifact_build_on_handler_exception decorator
was later re-raised and handled also by the fail_event_on_handler_exception
making the fail_artifact_build_on_handler_exception useless in this
combination of decorators.

This commit fixes this by changing the decorators to handle the
exception only once. That means only the first decorator will
handle the exception.

@yashn, the test change here is related to your original PR. I have found out there was one issue I've fixed here.

You originally raised fake exception directly from RebuildImagesOnParentImageBuild.update_db_build_state by patching it. It seems this is not correct, because the update_db_build_state is already decorated, so the patch method just patched that method including the decorator and the decorator was therefore never called.

I've changed that test to patch start_to_build_images instead, which is the method called in the update_db_build_state. That way, the decorator stays there and handles the exception raised by the start_to_build_images.

@jkaluza makes sense. Thanks!

Just to make sure I understand correctly, the change you added basically stops a decorator from handling the exception because some other decorator has already handled it right?

LGTM! :)

@yashn, yes, correct.

Commit 8b519496 fixes this pull-request

Pull-Request has been merged by jkaluza

Pull-Request has been merged by jkaluza

Metadata