1040 check coverage after getting files in get upstream dependency inputs #1041
Conversation
Add function for testing science file coverage
…ncy_inputs function.
…t_upstream_dependency_inputs
There was a problem hiding this comment.
Pull request overview
This PR adds coverage verification for upstream dependencies when require_coverage=True is set, ensuring complete date range or repoint coverage for spin-tables and science dependencies. This is intended to support multi-pointing coverage requirements for the Hi Goodtimes job.
Key Changes:
- Added two new coverage verification functions:
verify_spin_coveragefor spin files andverify_science_coveragefor science files - Modified
get_upstream_dependency_inputsto perform coverage checks whenrequire_coverage=True - Added extensive test coverage including unit tests for verification functions and integration tests for the full workflow
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
sds_data_manager/lambda_code/SDSCode/pipeline_lambdas/dependency.py |
Added verify_spin_coverage and verify_science_coverage functions; modified get_spin_files to return records instead of filenames; updated get_upstream_dependency_inputs and get_jobs to support coverage checking |
sds_data_manager/lambda_code/SDSCode/pipeline_lambdas/batch_starter.py |
Added require_coverage parameter to dependency calls with appropriate values (False for initial queries, True for job-specific queries) |
tests/lambda_endpoints/test_dependency_api.py |
Added comprehensive test coverage including unit tests for both coverage verification functions and integration tests for the complete workflow with various scenarios |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| f"Missing coverage for {len(sorted_missing)} dates ({missing_str})" | ||
| ) | ||
|
|
||
| logger.info(f"Incomplete science coverage for {dep_str}: {error_msg}") |
There was a problem hiding this comment.
Inconsistent logging levels: The docstring at line 610 states "If gaps are found, they are logged at WARNING level", but the actual implementation uses logger.info at lines 659 and 701 when gaps are found. This should use logger.warning to match both the docstring and the severity of the situation (incomplete coverage blocking processing).
| logger.info(f"Incomplete science coverage for {dep_str}: {error_msg}") | |
| logger.warning(f"Incomplete science coverage for {dep_str}: {error_msg}") |
|
|
||
| logger.info( | ||
| f"Science coverage verified for " | ||
| f"{dep_str}: {len(records)} file(s) provide daily coverage" |
There was a problem hiding this comment.
Misleading log message: When verifying repoint coverage (lines 662-665), the success message says "provide daily coverage", but in repoint mode the function is not checking daily coverage - it's only checking that specific repoints exist. The message should be updated to something like "provide repoint coverage" or "contain requested repoints" to accurately reflect what was verified.
| f"{dep_str}: {len(records)} file(s) provide daily coverage" | |
| f"{dep_str}: {len(records)} file(s) contain requested repoints" |
…t_upstream_dependency_inputs
1 participant
Change Summary
Overview
When reviewing comments on my Hi Goodtimes PR I realized that there was a deficiency in checking that proper coverage is provided by dependencies found in the
dependency.get_upstream_dependency_inputsfunction. This PR adds coverage checking for spin-tables and science dependencies when thecoverage_requiredflag is set. This should help with checking for the required multi-pointing coverage that will be required by the Hi Goodtimes job.See Copilot review for summary of changes.
Closes: #1040