Pass R errors through to Conbench #143
Conversation
| logging.exception(json.dumps(error)) | ||
| return error, output | ||
| result["error"]["command"] = r_command | ||
| logging.exception(f"Errored result (not posted): {json.dumps(result)}") |
There was a problem hiding this comment.
Theoretically we could post these to Conbench now, but I'm not sure whether there's value in doing so. Errors within the benchmark won't hit this—this is for when things bomb out entirely somehow, e.g. you spelled the name of the benchmark wrong.
There was a problem hiding this comment.
A lot of this is just rearranging; I moved the single-use assert_* function bodies directly into the tests so you can see what they're testing without navigating all over the code.
| assert output is None | ||
| @pytest.mark.parametrize("error_type", ["NULL"]) | ||
| @pytest.mark.parametrize("output_type", ["NULL", "'warning'"]) | ||
| def test_r_only_placebo(error_type, output_type): |
|
|
||
| @pytest.mark.parametrize("error_type", ["'base'"]) | ||
| @pytest.mark.parametrize("output_type", ["NULL", "'warning'"]) | ||
| def test_r_only_exception(error_type: str, output_type: str): |
There was a problem hiding this comment.
This is a significant refactor of an outdated test which no longer raised errors like before because arrowbench handles them properly. Now this tests that labs/benchmarks handles errors arrowbench catches properly.
Pull Request Test Coverage Report for Build 5340132227
💛 - Coveralls |
|
Ran against a local Conbench instance and it posts successfully: 
Tracebacks are too wide to read easily, and the warnings subdict doesn't get unpacked into a subtable, but these are existing UI issues. At least the data is there so users can copy it an reformat it to see what's going on. And we'll be able to see on Conbench when benchmarks are failing! |
|
The changes here are relatively simple and well-tested, so I'm going to merge this, but post-merge reviews are welcome; happy to do a follow-up is necessary. I'll keep an eye on buildkite to see if anything goes awry. |
Closes #134. Builds on voltrondata-labs/arrowbench#138 to take results with an
errorelement and pass it through to Conbench properly. Improves some tests along the way.Tests are passing locally, but I want to run this against a local Conbench instance to make sure nothing is malformed before merging.