handle frame in stack trace that doesn't have a module #1741
Conversation
facebook-github-bot
added
the
CLA Signed
|
Yeah, it's a hard one to test. I am actually not sure that the best handling is to break. |
|
you can test by triggering the code path to see what happens. |
|
I would say that in any case there is any exception at all while trying print a nice exception in that branch we should revert to printing the full exception. |
Previously `typing.Any` was used for the type of some Optional tracebacks in `hydra._internal.run_and_report`. This commit makes the typing more precise, using `types.TracebackType` and `Optional[types.TracebackType]` where appropriate. Additionally, a new type assertion is introduced based on feedback from mypy.
|
I wrapped the nice-exception logic in a try/except block. |
|
Can you check the CI failures? |
Ok, I've fixed most of the failures. |
tests/test_utils.py
Outdated
| class TestRunAndReport: | ||
| class DemoFunctions: | ||
| """Demo inputs for `run_and_report`""" |
There was a problem hiding this comment.
Can you explain the testing logic?
The test is surprisingly large.
There was a problem hiding this comment.
The test is surprisingly large.
Sorry about that.
This is not just testing the new functionality. It is also testing the pre-existing run_and_report behavior.
The run_and_report function has the following signature:
def run_and_report(func: Any) -> Any: ...
There are three test methods:
test_successtests whenrun_and_reportrunsfuncsuccessfully.test_failuretests whenfuncraises an exception andrun_and_reportis able to print a nicely-formatted error message.test_simplified_traceback_failuretests whenfuncraises an exception andrun_and_reportis not able to print a nicely-formatted error message -- this is the case where the original exception gets re-raised.
There are four @staticmethod definitions. These are passed in to run_and_report as the func argument. They are designed to trigger different logic code paths inside of run_and_report:
success_funcraises no exceptionsimple_errorraises anAssertionErrorrun_job_wrappertriggers the special logic inrun_and_reportthat looks for a function called"run_job"in the stack, and strips away the leading stack framesomegaconf_job_wrappertriggers the special logic inrun_and_reportthat looks for theomegaconfmodule in the stack, and strips away the bottom stack frames
There was a problem hiding this comment.
Gotcha.
Thanks for adding this. Can you add a comment describing this in the test class?
(does not need to be as detailed).
tests/test_utils.py
Outdated
| class TestRunAndReport: | ||
| class DemoFunctions: | ||
| """Demo inputs for `run_and_report`""" |
There was a problem hiding this comment.
Gotcha.
Thanks for adding this. Can you add a comment describing this in the test class?
(does not need to be as detailed).
|
Looking good, don't forget a bugfix news fragment. |
Closes #1739
Not sure how to test this.
Currently there are not any unit tests of the
run_and_reportfunction.