w0rd-driven
commented
1 year ago This is a big enough known issue and blocker tbh but I do want to release the repo anyway. What absolutely should be covered in this is to work through the Oban worker and the verify function. Why do I return a notification again? The broadcast sends it as an event and the record shouldn't do anything special.
It'll be good to have a full on refactor of this whole flow from top to bottom. The current UI having a verify() also save the artist record like the Oban job is awkward. I want to stick to SRP but I think prior languages are influencing how I break things up. Updating the timestamp is absolutely part of this call and I need to make sure data flows through the pipe in predictable ways.
Scanning is no longer idempotent or it possibly never was. After running a verify and a subsequent scan, I recently noticed how the images and
verified_attimestamps were removed. I had wondered why artist scans for id 4 would seemingly verify the entire collection again. It is problematic to use the worker that is very much specialized for a single backfill operation. It's not a bad thing to retry the failed lookups but that list will eventually grow indefinitely.I was able to reproduce this precisely and I have a feeling it comes back to the upsert operation. It's perfect for a single shot bulk insert but the timestamp placeholders create what look like new records instead of look for a whitelist. We only want it to update certain fields, not essentially roll back changes. This is a neat trick and would essentially count as starting over.