Conversation
There was a problem hiding this comment.
Code Review
This pull request integrates a new validation helper service into the import automation pipeline, adding build and deployment configurations for a Cloud Run job that runs a BigQuery-based dataset differ and an automated validation framework. The validation framework supports custom rules, SQL-based validations, and golden set checks. The review feedback identifies two important improvements: updating path resolution in the golden set validator to recursively handle lists of paths, and copying the initial dictionary in the MCF utility to prevent unintended in-place mutation of the caller's data.
| def _resolve_paths(path: str, config_dir: str) -> str: | ||
| """Resolves relative path to be absolute relative to config_dir.""" | ||
| if (isinstance(path, str) and path and not os.path.isabs(path) and | ||
| file_util.file_is_local(path)): | ||
| resolved = os.path.join(config_dir, path) | ||
| logging.info("Resolved relative path '%s' to '%s'", path, resolved) | ||
| return resolved | ||
| return path |
There was a problem hiding this comment.
The golden_files parameter can be specified as a list of paths (e.g., ["golden_data/critical_stats.csv"]). However, _resolve_paths only handles str inputs and will return a list unmodified without resolving its relative paths against config_dir. Update _resolve_paths to recursively handle lists of paths.
| def _resolve_paths(path: str, config_dir: str) -> str: | |
| """Resolves relative path to be absolute relative to config_dir.""" | |
| if (isinstance(path, str) and path and not os.path.isabs(path) and | |
| file_util.file_is_local(path)): | |
| resolved = os.path.join(config_dir, path) | |
| logging.info("Resolved relative path '%s' to '%s'", path, resolved) | |
| return resolved | |
| return path | |
| def _resolve_paths(path, config_dir: str): | |
| """Resolves relative path to be absolute relative to config_dir.""" | |
| if isinstance(path, list): | |
| return [_resolve_paths(p, config_dir) for p in path] | |
| if (isinstance(path, str) and path and not os.path.isabs(path) and | |
| file_util.file_is_local(path)): | |
| resolved = os.path.join(config_dir, path) | |
| logging.info("Resolved relative path '%s' to '%s'", path, resolved) | |
| return resolved | |
| return path |
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| UnusedCode | 7 medium 1 minor |
| Documentation | 48 minor |
| ErrorProne | 8 high |
| Security | 2 high |
| CodeStyle | 4 minor |
| Complexity | 30 medium |
🟢 Metrics 876 complexity
Metric Results Complexity 876
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.
Add
validation-helperCloud Run Job & Clean Up Differ/Validation UtilitiesSummary
Adds
validation-helperas a standalone Cloud Run Job (validation-helper-job) orchestrated byimport-automation-workflow.yamlto run dataset diffing and validation after Cloud Batch import jobs, and cleans up unused legacy code copied from thedatarepo.Changes
import-automation-workflow.yaml,build-services.yaml, anddeploy-services.yamlto runrunImportJobin execute-only mode and triggervalidation-helper-jobimmediately after.cloud) and DirectRunner (direct) modes fromimport_differ.py, keeping onlybigquery(default) andnativemodes. Extracted reusable MCF-to-BigQuery streaming/loading helpers intoutil/bq_util.py.tools/statvar_importer/(config_flags.py,mcf_diff.py,data_sampler.py), movingmcf_file_util.pytoutil/mcf_file_util.py.util/aggregation_util.pyandutil/config_map.py, and simplifiedutil/counters.py(removedpsutiland background timers).file_get_matching()inutil/file_util.pyto checkblob.exists()for non-wildcardgs://URIs.psutilandgoogle-api-python-clientdependencies frompyproject.toml.Testing
uv run pytest(120 passed).datcom-ci): Deployed and verifiedimport-automation-workflow-staging(1139ba22-f58b-4184-8bc2-92b2b365c941) onUSFed_ConstantMaturityRates_Test. Confirmedvalidation-helper-job-stagingran BigQuery differ + validation rules and uploadeddiffer_summary.csv,differ_summary.json, andvalidation_output.csvto GCS.