Conversation
|
|
@Niels-b another one changing the diagnostics, this time adds a new key. |
Niels-b
left a comment
There was a problem hiding this comment.
Adding new keys will not break the DWH. This just won't be included in the DWH diagnostics. Can always be changed later.
LGTM!
mivds
left a comment
There was a problem hiding this comment.
LGTM, just want to check off next steps:
- Is the intent to add support for these properties on BE?
- Do we need to support this in DWH? Would make a conscious decision here and create a ticket if yes, otherwise we'll forget about it until a customer complains about it
| "failedRowsPercent": check_result.diagnostic_metric_values.get("invalid_percent"), | ||
| "datasetRowsTested": check_result.diagnostic_metric_values.get("dataset_rows_tested"), | ||
| "checkRowsTested": check_result.diagnostic_metric_values.get("check_rows_tested"), | ||
| "missingCount": check_result.diagnostic_metric_values.get("missing_count"), |
There was a problem hiding this comment.
Not supported (yet) by BE from what I see. I think it will get quietly ignored rather than raise errors about unknown fields, but not 100% sure.
Same comment applies for duplicate check below
There was a problem hiding this comment.
BE ignores unknown fields, this PR was meant to be a draft, there is a ticket for BE to do follow up and will be first tested and then released next week
|
Yes indeed this was meant to be a draft PR. All context and planning in https://linear.app/sodadata/issue/DTL-1653/cbre-add-null-count-for-duplicate-and-validity-in-the-diagnostic |
|
merging after BE and FE now support this. Everything is working as expected |
Description
Please provide a description of your changes:
Adds missing count to validity check, reports it in diagnostics for missing and duplicate count.
Checklist