feat: support metrics-only writes without output or quarantine tables#1236
Conversation
|
|
mwojtyczka
left a comment
There was a problem hiding this comment.
Solid, well-scoped feature — and the tricky part (forcing an action so the Spark Observation is populated before Observation.get, which would otherwise block in metrics-only mode) is handled correctly. Three notes inline, plus two test gaps:
- No test for the
save_results_in_tableobserver-guard case flagged inline (an engine without an observer but given a validobservation+metrics_config). apply_checks_and_save_in_tables_for_patternsmetrics-only: thewrite_output/metrics_onlybranch changed but I don't see a for-patterns metrics-only test — worth confirming the per-matched-table metrics table is named/written correctly whenoutput_table_suffixis absent.
Not blocking beyond Finding 1.
mwojtyczka
left a comment
There was a problem hiding this comment.
Thanks for your contribution. The PR generally looks good to me. I left small comment.
…ment forced actions, add metrics-only tests - save_results_in_table no longer rejects a valid passed-in observation on an engine without an observer; save_summary_metrics builds the metrics frame via the static builder - both metrics_only_df.count() sites carry the reviewer's explanatory comment, extracted into _populate_batch_observation to keep McCabe under the pylint threshold (this was the fmt CI failure) - new integration tests: observer-less save_results_in_table and for-patterns metrics-only table naming without output_table_suffix
|
Thanks for applying the comment suggestions directly. The rest is addressed in 8674a01: dropped the observer guard in save_results_in_table per Finding 1 (save_summary_metrics now builds the metrics frame via the static builder, so a valid passed-in observation works on an engine without an observer), and added the two tests you flagged - observer-less save_results_in_table, and for-patterns metrics-only table naming when output_table_suffix is absent. One heads-up: the fmt CI failure was pylint R1260 (the new three-way guard pushed both apply_checks_*_save_in_table to McCabe 11), so I extracted it into a _populate_batch_observation helper - your comment text lives in its docstring. make test passes locally (1159 unit tests), pylint back at 10.00. |
|
@mvanhorn can you please sing the Contributor License Agreement |
ghanse
left a comment
There was a problem hiding this comment.
Overall LGTM, should we add another issue to allow users to configure metrics-only jobs via the installation wizard?
|
Thanks for merging, @mwojtyczka. Metrics-only writes without output or quarantine tables make pure-observability runs much cheaper. |
Changes
save_results_in_table()(and the corresponding workflow path) can now persist summary metrics without requiring an output or quarantine table. Previously a metrics-only configuration raised because the writer insisted on at least one results table; observability-focused pipelines that only want the metrics table had no supported path. Streaming and no-observer cases are validated with explicit errors, and batch observations are triggered before metrics are saved so the metrics table is populated correctly. Existing behavior with output or quarantine configs present is untouched; metrics-only is additive.Linked issues
Resolves #1216
Tests
Integration tests cover the new metrics-only path (with and without an explicit metrics config) and the updated error message for the no-argument call. The suite needs a Databricks workspace and runs in CI.
Documentation and Demos
quality_checks_apply.mdxandsummary_metrics.mdxnow describe the metrics-only mode.