Improve the output in case of errors #1586
Conversation
|
✅ 15/15 passed, 1 skipped, 10s total Running from acceptance #727 |
| header = f"/*\n Transpiled from {file_path}\n" | ||
| failed_producing_output = False | ||
| if errors: | ||
| header += "\n Following errors were found while transpiling:\n" |
There was a problem hiding this comment.
Better English:
The following errors....
| failed_producing_output = failed_producing_output or diag.kind == ErrorKind.PARSING | ||
|
|
||
| if failed_producing_output: | ||
| header += "\n\n Parsing errors prevented the converter to translate the input query.\n" |
There was a problem hiding this comment.
Better English:
...the converter from translating the input query.
| @@ -56,39 +56,76 @@ async def _process_one_file( | |||
|
|
|||
| output_path.parent.mkdir(parents=True, exist_ok=True) | |||
|
|
|||
| output_code = transpile_result.transpiled_code | |||
|
|
|||
| if any(err.kind == ErrorKind.PARSING for err in error_list): | |||
There was a problem hiding this comment.
How do we handle ErrorKind.Internal? the example of Recursive CTE?
There was a problem hiding this comment.
That is a good question. The if any(...) above detects cases where the output code is known to be incomplete (due to parse errors), which would therefore be useless to the user. In which case we display the input unchanged instead.
Other kinds of errors are dealt with the "regular" way, with the output displayed under the header. Are there other cases (apart from parsing errors) where we should display the input?
There was a problem hiding this comment.
I would (somewhat hypocritically) convert them to something like "Your source code relies on features not yet supported by Lakebridge. Contact support for help."
There was a problem hiding this comment.
@vil1 can you run the test on recursive CTEs it was a case where we are able to parse but not translate category, I m more concered about these as if we just put the same as source, people will assume code got translated
| @@ -56,39 +56,76 @@ async def _process_one_file( | |||
|
|
|||
| output_path.parent.mkdir(parents=True, exist_ok=True) | |||
|
|
|||
| output_code = transpile_result.transpiled_code | |||
|
|
|||
| if any(err.kind == ErrorKind.PARSING for err in error_list): | |||
There was a problem hiding this comment.
@vil1 can you run the test on recursive CTEs it was a case where we are able to parse but not translate category, I m more concered about these as if we just put the same as source, people will assume code got translated
* main: (57 commits) Improve the output in case of errors (#1586) Integrate analyzer (#1578) fix maven name for morpheus (#1585) Reduce logging noise during normal transpiler processing. (#1580) Improve logging during transpiler installation (#1576) LSP: include the client's process ID during the initialisation sequence (#1574) Ensure the Remorph CLI provides product information to the pluggable transpilers during initialisation. (#1568) Ensure test LSP server is shut down when tests fail (#1569) Wait for transpile capability during LSP initialisation (#1571) Integration Test with dedicated runner (#1573) Avoid adding unnecessary semicolons (#1572) Fix spark version detection in CI integration (#1566) Add Pipeline and Step Comment Attributes (#1557) Log lsp server stderr (#1471) Constrain the Databricks SDK to a single version (#1551) Mark `test_execute_query_without_connection` as allowed to fail locally if ODBC isn't available (#1550) Fix test temporary resources (#1548) Patches Transpiler Output Template (#1558) Create a pull request template we can use in all Remorph projects (#1545) Pluggable transpiler documentation fixes (#1532) ... # Conflicts: # Makefile # src/databricks/labs/remorph/cli.py # src/databricks/labs/remorph/reconcile/query_builder/hash_query.py # src/databricks/labs/remorph/reconcile/recon_capture.py # src/databricks/labs/remorph/reconcile/schema_compare.py # tests/unit/conftest.py # tests/unit/transpiler/test_execute.py # tests/unit/transpiler/test_lsp_engine.py
Changes
Add a header to the produced files that states from which input they were transpiled, and in case of error, summarize the errors. Errors are displayed with a position (line:column) in the output, which gets decorated with line numbers.
What does this PR do?
Relevant implementation details
Caveats/things to watch out for when reviewing:
Linked issues
Resolves #..
Functionality
databricks labs remorph ...Tests