Princeton-LSI-ResearchComputing / tracebase

Mouse Metabolite Tracing Data Repository for the Rabinowitz Lab
MIT License
4 stars 1 forks source link

Msruns loader rollback cleanup #983

Closed hepcat72 closed 3 months ago

hepcat72 commented 4 months ago

Summary Change Description

This code buffers created ArchiveFile records and very cautiously deletes the files in the archive that were created in the event of a rollback.

The issue originally was to only exit if the default sequence records could not be found, but I quickly realized that there are all sorts of common exceptions that could happen and leave behind orphaned files: missing sample record (very common), missing sequence detailed in the infile (common), duplicates, missing headers... the list goes on and on. And when I started thinking about having the add a front-loaded loop to handle the infile exceptions for missing sequence records, I started to think of the strategy and eventually creating messy spaghetti code, so I decided a cleaner solution was to simply rollback the directory using a buffer of created ArchiveFile records.

I pointed out in the original PR though that there's a potentially better strategy that avoids file deletion: buffer the creation of the files on_commit after a post_save signal (for each record). I have to think more about this, but in theory, it should work, and be a more comfortable change for those that worry about unintended deletions. however, I for one am very confident in this code. I think that post-save-on-commit would be better, because it's more efficient, but it's the same basic mechanism, which was intended for this express purpose. We should use it IMO.

Affected Issues/Pull Requests

Review Notes

See comments in-line.

Checklist

This pull request will be merged once the following requirements are met. The author and/or reviewers should uncheck any unmet requirements: