Added goldens for USCensuspep imports#2091
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces validation configurations, golden summary reports, and generated outputs for the US Census PEP monthly population estimate and sex datasets. Additionally, it updates download routines in process.py to include timeouts, retries with exponential backoff, and sorted file processing. Feedback on the changes highlights an incorrect golden file path in the us_pep_sex validation configuration and a potential resource leak in process.py where temporary files are not cleaned up if an exception occurs during file copy or move operations.
| "rule_id": "check_goldens_summary_report", | ||
| "validator": "GOLDENS_CHECK", | ||
| "params": { | ||
| "golden_files": "../../../../golden_data/golden_summary_report.csv" |
There was a problem hiding this comment.
The path ../../../../golden_data/golden_summary_report.csv points to a golden file that does not exist for this import, or it incorrectly references a shared path. Since us_pep_sex is a different dataset from monthly_population_estimate, it should have its own golden summary report (e.g., generated from scripts/us_census/pep/us_pep_sex/output/new_rec_gen_sa/summary_report.csv and placed in a golden_data directory under us_pep_sex). Note that the path in the configuration should still be specified as a relative path from the execution context (using ../ relative pathing) rather than relative to the configuration file itself.
References
- Golden file paths in validation configurations should be specified as relative paths from the execution context (using relative pathing like
../) rather than relative to the configuration file itself.
| with tempfile.NamedTemporaryFile(delete=False) as tmp_file: | ||
| for chunk in response.iter_content(chunk_size=8192): | ||
| if chunk: | ||
| tmp_file.write(chunk) | ||
| tmp_file_path = tmp_file.name | ||
|
|
||
| # Copy to local destination | ||
| shutil.copy( | ||
| tmp_file_path, | ||
| os.path.join(_INPUT_FILE_PATH, file_name_to_save)) | ||
|
|
||
| # Move to gcs destination (optimized from shutil.copy + os.remove) | ||
| shutil.move( | ||
| tmp_file_path, | ||
| os.path.join(_GCS_FOLDER_PERSISTENT_PATH, | ||
| file_name_to_save)) |
There was a problem hiding this comment.
If an exception occurs during shutil.copy or shutil.move, the temporary file created by tempfile.NamedTemporaryFile(delete=False) will not be cleaned up, leading to orphaned files in the temp directory. Wrap the file operations in a try...finally block to ensure the temporary file is always deleted.
tmp_file_path = None
try:
with tempfile.NamedTemporaryFile(delete=False) as tmp_file:
for chunk in response.iter_content(chunk_size=8192):
if chunk:
tmp_file.write(chunk)
tmp_file_path = tmp_file.name
# Copy to local destination
shutil.copy(
tmp_file_path,
os.path.join(_INPUT_FILE_PATH, file_name_to_save))
# Move to gcs destination (optimized from shutil.copy + os.remove)
shutil.move(
tmp_file_path,
os.path.join(_GCS_FOLDER_PERSISTENT_PATH,
file_name_to_save))
finally:
if tmp_file_path and os.path.exists(tmp_file_path):
try:
os.remove(tmp_file_path)
except Exception:
pass
No description provided.