Fix source blob backup ETag comparison failing every blob

[Paul Lizer (paullizer)]

Fixes #1271

Problem

Every source blob in every Data Management backup failed with Source blob changed while it was being backed up. — 100% failure rate, zero retries, across all containers. User documents, group documents, public documents, and chat attachments have never been backed up. The job still reported completed_with_warnings, which made it easy to miss.

From job data_management_partial_20260818T0300Z:

Container	Blobs read	Copied	Failed
user-documents	71	0	71
group-documents	427	0	427
public-documents	95	0	95
personal-chat	19,394	0	19,394

[DATA_MANAGEMENT] Source blob backup completed with failures. -- {'container': 'personal-chat',
 'failed_count': 19394, 'copied_count': 0, 'source_read_count': 19394,
 'failure_reasons': '19394x Source blob changed while it was being backed up.'}

Root cause

_transfer_backup_source_blob compares two ETags that come from different Azure SDK code paths with different transport formats:

Value	Origin	SDK path	Format
source_etag	list_blobs()	get_blob_properties_from_generated_code() reads the XML <Etag> element	0x8DE...
current_source_etag	get_blob_properties()	BlobProperties(**headers) reads the HTTP ETag header	"0x8DE..."

The HTTP ETag header is an RFC 7232 quoted-string; the List Blobs XML element is not. azure-storage-blob==12.24.1 normalizes neither — I grepped the installed package to confirm. So the check was always 0x8DE... != "0x8DE...".

RuntimeError is not retryable under _is_retryable_backup_blob_error, so every blob failed on the first attempt — matching the observed Retries / throttles: 0 / 0.

Independent confirmation the blobs did not actually change: ranged reads send that same unquoted ETag as an If-Match precondition and Azure accepted them. Only the Python-side comparison was wrong. The guard also runs after staging and commit, so every blob was fully downloaded, encrypted, and uploaded before being discarded — and left behind as an orphaned pending artifact.

Changes

1. ETag normalization — new _normalize_backup_etag strips transport quoting and the optional W/ weak-validator prefix, applied only at the comparison site.

source_item["source_etag"] deliberately keeps the exact value returned by list_blobs(), so the If-Match precondition sent to Azure is byte-for-byte unchanged from current production behavior. The guard is preserved — a genuine mid-transfer change still fails.

2. Checkpoint batching — record_transfer_result previously called persist() per item: 19,394 Cosmos writes for one container, capping throughput near 6 items/sec and stretching the run to 74 minutes. New maybe_persist checkpoints on manifest batch size (100) or a 15-second interval, whichever comes first. The job lease is still asserted every item, and the existing tail persist() still flushes the final partial batch. Worst-case re-work after an interrupted run stays bounded at 100 items or 15 seconds.

3. Functional tests that silently passed under pytest — while validating this fix I found the repo's standard test template hides failures:

def test_x():
    try:
        assert ...
        return True
    except Exception:
        return False

Returning a value instead of raising makes pytest report the test as passed with only a PytestReturnNotNoneWarning. A deliberately broken build reported 7 passed while the transfer was genuinely failing. Both backup test files now assert directly and return None, with the __main__ block preserving standalone output and exit codes. Suite warnings dropped 13 → 1.

Scope check

The migration path (_copy_source_blobs_to_target) has a visually identical comparison, but sources source_properties from get_blob_properties(), so both operands are already quoted. Migration is not affected and is left unchanged.

Validation

New test functional_tests/test_data_management_backup_source_blob_etag.py drives the real _transfer_backup_source_blob against in-memory blob clients with an unquoted listing and a quoted fetch.

7 passed in 2.51s

Test	Assertion
test_etag_normalization_strips_transport_quoting	Quoted, unquoted, weak, padded ETags normalize identically
test_listed_and_fetched_etags_compare_equal	Raw listed ETag preserved for If-Match; both formats equal once normalized
test_transfer_succeeds_across_list_and_get_etag_formats	End-to-end transfer succeeds and promotes metadata to succeeded
test_genuinely_changed_source_blob_still_fails	ETag changing after download is still rejected
test_verified_artifact_matches_source_version	Reuse detection still keys off recorded source version
test_checkpoint_interval_is_bounded	Interval and batch size within safe bounds
test_version_is_at_least_fix_version	Version floor

Regression probe: neutralizing _normalize_backup_etag fails 4 of 7 tests, including the end-to-end transfer, reporting the exact production message:

Transfer must succeed, got 'failed' ('Source blob changed while it was being backed up.')

Full Data Management suite: 154 passed, 1 failed. That failure (test_backup_recovery_and_admin_progress_are_bounded_and_sanitized) was confirmed pre-existing on origin/Development in the previous PR and is unrelated.

Version

0.250.217 → 0.250.218

Follow-up

#1272 tracks the cosmetic issue where completed jobs still display Current container: Waiting and running step badges.

Documentation

• docs/explanation/fixes/SOURCE_BLOB_BACKUP_ETAG_FIX.md
• docs/explanation/release_notes.md

[Paul Lizer (paullizer)]

Copilot resolve the merge conflicts in this pull request

[Copilot]

Copilot resolve the merge conflicts in this pull request

Merge conflicts resolved in 96d7c26.

Only docs/explanation/release_notes.md conflicted. Development had meanwhile shipped its own v0.250.218 section (credential key redaction, CosmosClient import bindings, privacy logging audit), so I kept that section intact and moved this PR's three entries into a new v0.250.219 section above it.

Because Development claimed 0.250.218, this fix is renumbered to 0.250.219 in config.py, SOURCE_BLOB_BACKUP_ETAG_FIX.md, and the test header/version floor. No production logic changed in the merge — _normalize_backup_etag and the batched checkpointing are unchanged.

Verified after merging: test_data_management_backup_source_blob_etag.py is 7/7 and test_data_management_backup_cosmos_pagination.py is 4/5, the one failure being a missing azure.monitor package in this sandbox rather than a code issue.

[Paul Lizer (paullizer)]

Copilot resolve the merge conflicts in this pull request

[Copilot]

Copilot resolve the merge conflicts in this pull request

Resolved again in 1792dd9 — Development moved forward with #1274, which claimed v0.250.219, the number this PR took in the previous round.

Same resolution as before: only release_notes.md conflicted. I kept #1274's v0.250.219 section (agent document search citations, plus its UI enhancement entry) intact and moved this PR's three entries into a new v0.250.220 section above it. Renumbered 0.250.219 → 0.250.220 in config.py, SOURCE_BLOB_BACKUP_ETAG_FIX.md, and the test header/version floor, leaving #1274's files at 0.250.219.

functions_data_management.py merged cleanly — the diff against Development is still only my six files, with _normalize_backup_etag and the batched checkpointing unchanged.

Validation after the merge (installed the missing test deps this time, so the previously unrunnable tests actually execute):

• test_data_management_backup_source_blob_etag.py — 7/7
• test_data_management_backup_cosmos_pagination.py — 5/5 (was 4/5 last round only because azure.monitor was missing)
• test_agent_document_search_citations.py — 12/12, confirming Cite documents agents find via document search #1274's version floor still holds at 0.250.220