Reset MARC files on settings update (PP-4637)

[tdilauro]

Description

• Adds a marc_export_reset Celery task that sets up the next MARC export as a first run by removing existing files (database records and S3 objects). The records are deleted in one transaction, and S3 deletion is best-effort, so a failure won't leave the reset half-applied.
• Queues the reset when a library's MARC exporter settings change in the admin interface and when a library registry pushes a new web client URL (only if the change would result in a new web client URL).
• First-run full exports now also record a full-content delta pointing at the same S3 object, so (1) consumers that only apply delta files receive the complete refreshed dataset without a second upload and (2) we avoid an extra upload and the doubled storage utilization.
• Export finalization now detects a configuration change that raced an in-flight export (records reset, settings drift, or the library no longer MARC-enabled), discards the stale upload, and immediately re-queues a fresh full export.
• Cleanup and reset only delete a shared S3 object when the last record referencing it is removed, and upload aborts are best-effort so cleanup failures cannot cascade.
• The MARC download page labels the full-content delta as "Full content as of " instead of a date range starting at the full delta sentinel epoch.

Motivation and Context

MARC record exports run periodically, but a configuration change would not trigger an update. Record updates based on those changes might not become available until long after the change was made. Further, some MARC consumers apply only delta files rather than reprocessing full exports. When a library's MARC export configuration changed (organization code, summary/genre inclusion, web client URLs), records exported before the change were never corrected for those consumers, leaving stale data in their catalogs indefinitely. Resetting the library's export state on configuration changes forces a full refresh that reaches both full-file and delta-only consumers.

[Jira PP-4637]

How Has This Been Tested?

• New and updated tests for the new functionality.
• All tests/checks pass locally.
• CI tests/checks pass.

Checklist

• I have updated the documentation accordingly.
• All new and existing tests passed.

[claude]

Claude finished @tdilauro's task in 9m 55s —— View job

---

Code Review

I reviewed the reset task, the in-flight drift detection (_finalize_uploads / _verify_uploads_after_commit), the shared-key cleanup/reference-counting, the full-content delta mechanism, and the admin + registry enqueue paths.

Summary

This is a carefully-constructed change and it holds up under scrutiny. I verified the parts most likely to hide a defect and they're correct: S3 keys embed a uuid4 (uploader._s3_key), so the only records that ever share a key are the intended first-run full + full-content-delta pair — which makes _marc_file_key_ref_count and the reset's still_referenced guard sound; MarcFile.since is DateTime(timezone=True) and MARC_EPOCH is tz-aware, so update.since == MARC_EPOCH compares instants correctly and the download label works; delete_files_for_reset scoping, the DELETE … RETURNING correspondence, and the records_gone / settings-drift checks all line up with the two-phase (pre-commit finalize + post-commit verify) race handling. The commit-ordering, best-effort-S3, and dispatch-before-commit tradeoffs are deliberate, documented in-code, and were already discussed with the earlier reviewers, so I won't re-litigate them. Test coverage is thorough (race variants, library-disabled, shared-key cleanup, S3-failure, registry web-client matrix). I found no blocking issues and nothing that meets the bar for an actionable change.
· chore/handle-marc-export-config-update

[greptile-apps]

Greptile Summary

This PR resets MARC export state when export-affecting settings change. The main changes are:

• Adds a MARC reset task that removes database records and best-effort deletes S3 objects.
• Queues resets from admin settings updates and registry web-client URL changes.
• Records a first-run full-content delta that points at the full export object.
• Detects in-flight export drift and requeues fresh full exports when needed.
• Updates cleanup and download-page handling for shared full/delta objects.

Confidence Score: 5/5

This looks safe to merge.

• No blocking issues found in the changed code.

Important Files Changed

Filename	Overview
src/palace/manager/celery/tasks/marc.py	Adds reset, drift detection, post-commit verification, shared full-content delta records, and shared-key-aware cleanup.
src/palace/manager/integration/catalog/marc/exporter.py	Adds reset deletion helpers, MARC epoch handling, URL deduplication, and web-client reset checks.
src/palace/manager/api/admin/controller/catalog_services.py	Queues MARC resets when existing library exporter settings change.
src/palace/manager/integration/discovery/opds_registration.py	Queues MARC resets when registry web-client URL changes affect MARC output.
src/palace/manager/api/controller/marc.py	Labels full-content delta downloads without showing the sentinel date range.
src/palace/manager/integration/catalog/marc/uploader.py	Makes discarded multipart upload aborts best-effort.

_{Reviews (8): Last reviewed commit: "Code review feedback" | Re-trigger Greptile}

[codecov]

Codecov Report

❌ Patch coverage is 98.43750% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.51%. Comparing base (3a39802) to head (6b28aee).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
src/palace/manager/celery/tasks/marc.py	98.82%	0 Missing and 1 partial ⚠️
...manager/integration/discovery/opds_registration.py	88.88%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3554      +/-   ##
==========================================
+ Coverage   93.50%   93.51%   +0.01%     
==========================================
  Files         512      512              
  Lines       46640    46754     +114     
  Branches     6358     6378      +20     
==========================================
+ Hits        43612    43724     +112     
  Misses       1959     1959              
- Partials     1069     1071       +2     

☔ 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.

[jonathangreen]

Looks good! A couple comments to think about, but this could be merged as is.

[jonathangreen]

You might want to add a cooldown here. I've been bitten before where, surprisingly (to me at least), the background task starts before the transaction commits, so it gets old settings.

[tdilauro]

I wasn't too worried about the settings here, since this reset is just deleting the records. The reset task itself doesn't read settings, so running early wouldn't feed it stale data. But the cooldown does fix another problem. We queue the reset just before saving the settings change, so it was possible for the reset to run before the save landed. We could (if we were really unlucky) have an export that was finishing at that exact moment that could have its files survive our checks and keep content generated with the old settings available (I went back and forth with the AI reviewers about this). So, the cooldown will help ensure that the reset always runs after the settings are saved, which is what our checks assume.

[jonathangreen]

Do we want to add an explicit commit here? I think you can rely on the commit done by the controller instead of adding another here.

[tdilauro]

I'm adopting the cooldown from your previous comment, so it should be safe to remove this commit.

[jonathangreen]

Minor: Might want to consider using json_canonical util (or maybe some generalized version) for this.

[tdilauro]

Thanks for the tip! 🎉