Repository navigation
[PreProcessor] Report config errors through the normal failure path - #844
Conversation
A bad config.json used to fail in Runner.__init__, before run()'s failure handling, so it wrote no report.json, no ingestion report and no workflow handshake: the ingestion workflow got no structured error. Runner now sets up the output store in __init__ and reads the config at the start of run(). A config that fails to load or validate is reported like any other failed run, with the same report.json, ingestion report and FAILURE handshake shapes. Config errors carry the INVALID_CONFIGURATION type; a missing config file stays a GENERIC_ERROR. run() now uses an error's error_type for the handshake instead of always GENERIC_ERROR, so MetadataValidator errors are reported as INVALID_CONFIGURATION, MISSING_PROVENANCE and MISSING_SOURCE as intended.
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| Documentation | 8 minor |
| CodeStyle | 1 minor |
🟢 Metrics 4 complexity
Metric Results Complexity 4
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the runner and configuration parsing to ensure that configuration errors are properly caught and reported as structured INVALID_CONFIGURATION failures rather than generic errors. It updates the initialization order in Runner to set up output directories and reporters before loading the configuration, and updates corresponding test cases to assert the correct handshake error types. The review feedback suggests a cleaner, more type-safe approach to error handling by defining a custom ConfigurationError exception class instead of dynamically attaching custom attributes to standard built-in exceptions like ValueError and JSONDecodeError.
Today a bad
config.jsonfails inRunner.__init__, beforerun()'s failure handling, so it writes noreport.json, no ingestion report and no handshake. The ingestion workflow gets no structured error.Changes
Runner.__init__only sets up the output store, reporter and summary generator.run()calls the new_load_config()inside its existingtry, which reads the config and opens the input stores (through a small_open_input_storehelper). A config failure therefore produces the samereport.json,ingestion_report.jsonand{"status": "FAILURE", "errors": [...]}handshake as any other failure. No schema or field changes.dataDownloadUrl) carryINVALID_CONFIGURATION. A missing config file staysGENERIC_ERROR.run()usese.error_typewhen set, instead of alwaysGENERIC_ERROR. This changes the reported type of three existing failures:GENERIC_ERRORMISSING_PROVENANCEGENERIC_ERRORMISSING_SOURCEGENERIC_ERRORINVALID_CONFIGURATIONVerification
Six scenarios ran through the real
Runner(testsuite harness:WORKFLOW_EXECUTION_IDset, output and handshake on fake GCS), on master and on this PR:FAILURE,INVALID_CONFIGURATIONFAILURE,INVALID_CONFIGURATIONFAILURE,GENERIC_ERRORGENERIC_ERRORMISSING_PROVENANCE(only change)FAILURE,INVALID_CONFIGURATION, message names the subdir confighandshake_error_types=()expectations now expectINVALID_CONFIGURATION(orGENERIC_ERRORfor missing files). New:test_invalid_config_is_reported_like_any_failed_runpins the handshake, report.json and ingestion report for a bad subdirectory config. Lint clean; full suite passes.Stack: this PR → #845