Skip to content

Log deleted-record percentages per import input - #2189

Open
rohitkumarbhagat wants to merge 5 commits into
datacommonsorg:masterfrom
rohitkumarbhagat:validation-deletion-metrics
Open

Log deleted-record percentages per import input#2189
rohitkumarbhagat wants to merge 5 commits into
datacommonsorg:masterfrom
rohitkumarbhagat:validation-deletion-metrics

Conversation

@rohitkumarbhagat

Copy link
Copy Markdown
Contributor

What changed

  • Add deleted-record percentage, deleted-record count, and previous observation count to validation metrics.
  • Emit validation status independently for each import input.
  • Include the import input identifier in each validation metric.
  • Prevent malformed or unknown validation rules from causing metric logging to fail.
  • Add coverage for multiple inputs with mixed validation statuses.

Tests

  • Focused import executor unit test
  • Google YAPF formatting check
  • Python compilation
  • git diff --check

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request enhances metric logging during import validation by extracting and reporting specific metrics (such as deleted records percentage, deleted records count, and previous observation count) when the DELETED_RECORDS_PERCENT validator is used. It also adds a comprehensive unit test to verify this behavior. The review feedback highlights two important issues: first, a potential runtime error if validation rules are malformed, which can be resolved by adding explicit type checks when parsing rules; second, a logical bug where input_validation_status is not reset to False inside the except ValueError block, potentially leading to incorrect success status logging when a validation runner fails.

Comment thread import-automation/executor/app/executor/import_executor.py Outdated
Comment thread import-automation/executor/app/executor/import_executor.py
Comment thread import-automation/executor/app/executor/import_executor.py Outdated
 Include per-input rule details with safe serialization and bounded payload size.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants