Skip to content

fix(sync): avoid UnboundLocalError in the upload_file error handler - #369

Open
devgtv wants to merge 1 commit into
AlertaDengue:mainfrom
devgtv:fix/sync-upload-file-unbound-raw-path
Open

devgtv wants to merge 1 commit into
AlertaDengue:mainfrom
devgtv:fix/sync-upload-file-unbound-raw-path

Conversation

@devgtv

@devgtv devgtv commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

fix(sync): avoid UnboundLocalError in the upload_file error handler

upload_file()'s except BaseException handler cleans up both the raw
download and the converted parquet, but only parquet_file was
pre-initialised. raw_path is first bound at the _download_raw_with_retry
call, so any failure raised before that point made the handler itself
raise UnboundLocalError: cannot access local variable 'raw_path'.

That replaced the real exception with a misleading message, since
_reprocess surfaces whatever the handler propagates — the operator saw
"cannot access local variable 'raw_path'" instead of the actual cause.
The temp file was also orphaned, as the cleanup aborted before running.

Both names are now bound to None before the try block and the handler
guards on raw_path, matching the existing parquet_file handling. Failures
before the download now propagate their original type and message.

Tests cover both paths: an error raised by _ensure_management_columns
(before the download) propagates unchanged, and an error raised while
hashing the download (after it) still removes the temp file.


This is a re-submission of #350, which was closed without review. The
workflow runs on that PR sat in action_required for three days and then
expired unapproved, so CI never went green.

This workflow run required approval but was not approved before it expired.

The fix is unchanged; the branch was rebased onto the current main, where
the suite passes (1780 passed, 6 skipped) and black/isort/flake8 are clean.

upload_file()'s `except BaseException` handler cleans up both the raw
download and the converted parquet, but only parquet_file was
pre-initialised. raw_path is first bound at the `_download_raw_with_retry`
call, so any failure raised before that point made the handler itself
raise `UnboundLocalError: cannot access local variable 'raw_path'`.

That replaced the real exception with a misleading message, since
`_reprocess` surfaces whatever the handler propagates — the operator saw
"cannot access local variable 'raw_path'" instead of the actual cause.
The temp file was also orphaned, as the cleanup aborted before running.

Both names are now bound to None before the try block and the handler
guards on raw_path, matching the existing parquet_file handling. Failures
before the download now propagate their original type and message.

Tests cover both paths: an error raised by _ensure_management_columns
(before the download) propagates unchanged, and an error raised while
hashing the download (after it) still removes the temp file.
@codecov-commenter

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (main@9db5915). Learn more about missing BASE report.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #369   +/-   ##
=======================================
  Coverage        ?   97.20%           
=======================================
  Files           ?      180           
  Lines           ?    23310           
  Branches        ?        0           
=======================================
  Hits            ?    22659           
  Misses          ?      651           
  Partials        ?        0           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

This branch has not been deployed

No deployments
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